Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break long input lines #263

Closed
wants to merge 1 commit into from
Closed

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Apr 3, 2024

Make it easier to read text that is typed in.

If this pull request addresses an open issue on the repository, please add 'Closes #NN' below, where NN is the issue number.

Closes #262

Please briefly summarise the changes made in the pull request, and the reason(s) for making these changes.

Make it easier to read entered text

If any relevant discussions have taken place elsewhere, please provide links to these.

Make it easier to read text that is typed in.
Copy link

github-actions bot commented Apr 3, 2024

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/LibraryCarpentry/lc-shell/compare/md-outputs..md-outputs-PR-263

The following changes were observed in the rendered markdown documents:

 06-free-text.md | 30 ++++++++++++++++++++----------
 md5sum.txt      |  2 +-
 2 files changed, 21 insertions(+), 11 deletions(-)
What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2024-04-03 17:21:41 +0000

github-actions bot pushed a commit that referenced this pull request Apr 3, 2024
@kaitlinnewson
Copy link
Member

Hello @bkmgit, thanks for your pull request. Unfortunately we won't merge this, as it does not give us the correct output. Breaking into multiple lines also makes it harder for learners to copy and paste commands when they need to.

Here's an example screenshot of one of the rendered changes for reference:
Screenshot 2024-04-08 at 9 16 02 AM

@bkmgit
Copy link
Contributor Author

bkmgit commented Apr 9, 2024

@kaitlinnewson Thanks for considering and reviewing it. Generally copy pasting is discouraged as it does not build muscle memory. The text that would be entered would be

tr -d '[:punct:]\r' < gulliver-noheadfoot.txt > \
gulliver-noheadfootpunct.txt

but typically the terminal also contains a $ and > which the user does not enter. Is the output wrong when this is done? Keeping the line length short is done in a few other curricular that use the shell, so if it is confusing, it would be good to know how it could be improved.

@kaitlinnewson
Copy link
Member

@bkmgit The commands as they are written in the PR result in a duplicate ">" character, one on each line, as you can see in the screenshot above that shows the rendered lesson content.

I took a look at some of the other lessons in the Carpentries materials, and breaking long commands on to multiple lines isn't being done elsewhere - I think one reason is that it adds complexity to teach the learner about breaking commands on to multiple lines, and there are no guarantees about the screen size a user has. We want to ensure that there is consistency across the lesson materials.

While I know that copying and pasting isn't ideal, we still want to make sure learners are able to do that easily in cases where they need to - and instructors may need to do that easily as well while they are teaching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Break long lines with commands
2 participants