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

Remove prefixed $ from shell command examples #610

Merged

Conversation

larouxn
Copy link
Contributor

@larouxn larouxn commented Apr 12, 2022

As the prefixed $ makes it impossible to copy and paste example shell commands, I was thinking it might be beneficial to remove them from the README. What do y'all think?

The prefixed $ makes it impossible to simply copy and paste the examples.
@larouxn larouxn added the documentation Improvements or additions to documentation label Apr 12, 2022
@larouxn larouxn self-assigned this Apr 12, 2022
Copy link
Contributor

@nvasilevski nvasilevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did a very similar thing for one of our internal docs so I think this change makes total sense 👍
Just in case let's wait for someone else's opinion

Copy link
Contributor

@adrianna-chang-shopify adrianna-chang-shopify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, LGTM! :shipit:

@etiennebarrie
Copy link
Member

I don't know because while it makes copying more easy with triple-click selection, it also makes it less clear that these are commands to type at the shell prompt… Also it's the default from bundler generated gems for instance, so I don't think people are too confused:
https://github.com/rubygems/rubygems/blob/085094b60caa7a9ac6771a7fc6b452fc79911bfb/bundler/lib/bundler/templates/newgem/README.md.tt#L11
https://github.com/rails/rails/#getting-started

@larouxn
Copy link
Contributor Author

larouxn commented Apr 12, 2022

I suppose it is a bit less clear they are shell commands but I feel it's still somewhat clear given the code block formatting and usage of terms like bin/rails. As for Bundler's default, a bit brazen, but I feel Bundler should update their docs/generation as well then. Ever since GitHub added this copy button, having breaking $ prefixed has become a bit of an issue.
Screen Shot 2022-04-13 at 7 30 28

re: triple click, this is the current experience. Can't do much with $ gem install rails. 🤷‍♂️
Screen Shot 2022-04-13 at 7 35 35

I don't think this change makes things more clear. Perhaps 20% less clear but 100% more convenient. 😅

Copy link
Member

@etiennebarrie etiennebarrie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's what I meant by triple-clicking, this PR makes it much more useful. The copy button is similar in that regard. Ok I'm convinced!

While we're at it, can we change all the bash into sh-session?

@larouxn
Copy link
Contributor Author

larouxn commented Apr 14, 2022

While we're at it, can we change all the bash into sh-session?

Sure thing. Done!

(feel free to merge this if it looks good on your end)

@etiennebarrie etiennebarrie merged commit 1446aba into main Apr 14, 2022
@etiennebarrie etiennebarrie deleted the remove_prefixed_dollar_signs_from_shell_command_examples branch April 14, 2022 09:08
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems May 6, 2022 20:12 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems-v1 May 9, 2022 12:49 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants