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 leading "$ " from copy-pastable commands #20

Merged
merged 2 commits into from
Mar 15, 2023

Conversation

deivid-rodriguez
Copy link
Contributor

When showing both the command and logs, the leading dollar makes things
more readable. However, in snippets with individual commands it doesn't
make it look better, and it's more useful without since it makes the
command directly pasted in the terminal work.

@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner October 13, 2022 08:10
Copy link
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@mattt mattt left a comment

Choose a reason for hiding this comment

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

I prefer to have a leading $ to indicate a terminal prompt (# for root) first to distinguish it from normal code, and second because it helps distinguish results of running the command in subsequent lines.

The downside is that you can't copy-paste directly the whole block into a terminal, but maybe that's enough of a speed bump for folks to be more careful about what they commands they're running.

Since we have both commands with and without output, I'd prefer to keep a consistent style throughout, which means a leading $.

I adopted this convention working as a technical writer, where this was the established guideline in the department. For what it's worth, this convention is adopted in GitHub docs. But if a majority of us want to use a different convention here, I'll defer to the group's preferences.

@jeffwidman
Copy link
Member

jeffwidman commented Oct 13, 2022

I have a slight preference for retaining the leading $ for all the reasons @mattt mentions ☝️.

But it's not something I feel strongly on, so thought if someone cared enough to open a PR, they probably care about this more than my level of preference, so that's why I was comfortable approving the PR.

tldr, I'm fine either way, but if you really want to know I do slightly lean toward retaining it.

@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented Oct 13, 2022

they probably care about this more than my level of preference,

No, not really!

I just have the habit of making this contribution whenever I copy a GitHub snippet with the "copy to clipboard" button and end up running a command starting with $, I don't think I'm getting more careful with time haha 😅.

I'll just close this given not everybody agrees!

@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/easier-copy-pasting branch October 13, 2022 17:27
@deivid-rodriguez deivid-rodriguez restored the deivid-rodriguez/easier-copy-pasting branch March 9, 2023 22:43
When showing both the command and logs, the leading dollar makes things
more readable. However, in snippets with individual commands it doesn't
make it look better, and it's more useful without since it makes the
command directly pasted in the terminal work.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/easier-copy-pasting branch from 9b57ace to 2467884 Compare March 9, 2023 22:44
@deivid-rodriguez
Copy link
Contributor Author

Just saw #74. The team seems now more aligned towards favoring easy copy-pasting, so I reopened this!

@jakecoffman jakecoffman enabled auto-merge (squash) March 15, 2023 18:50
@jakecoffman jakecoffman merged commit 997ba0d into main Mar 15, 2023
@jakecoffman jakecoffman deleted the deivid-rodriguez/easier-copy-pasting branch March 15, 2023 18:51
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.

4 participants