Skip to content

INSTALL.md: Restore $ as command prompt indicator#12257

Closed
levitte wants to merge 1 commit intoopenssl:masterfrom
levitte:restore-dollar-in-INSTALL.md
Closed

INSTALL.md: Restore $ as command prompt indicator#12257
levitte wants to merge 1 commit intoopenssl:masterfrom
levitte:restore-dollar-in-INSTALL.md

Conversation

@levitte
Copy link
Member

@levitte levitte commented Jun 24, 2020

We have a notational convention in INSTALL.md, which says this among
others:

Any line starting with a dollar sign is a command line.

$ command

The dollar sign indicates the shell prompt and is not to be entered as
part of the command.

That notation exists to make it clear what is a command line and
what's output from that command line.

@levitte
INSTALL.md: Restore $ as command prompt indicator
e6dbc95 
We have a notational convention in INSTALL.md, which says this among
others:

> Any line starting with a dollar sign is a command line.
>
>     $ command
>
> The dollar sign indicates the shell prompt and is not to be entered as
> part of the command.

That notation exists to make it clear what is a command line and
what's output from that command line.
@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Jun 24, 2020
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jun 24, 2020
@richsalz
Copy link
Contributor

richsalz commented Jun 24, 2020

Does OpenSSL have the issue in other places, like manpages? "git grep '^ '" might be useful.

@DDvO DDvO mentioned this pull request Jun 24, 2020
Closed
@DDvO
Copy link
Contributor

DDvO commented Jun 24, 2020 •
edited
Loading

Repeating some comments of mine that apparently were ignored being in https://github.com/openssl/openssl/pull/12175/files#r444780398 and below it:

I am not happy about that $ notation, mostly because when there is a number of shell commands in a row, such as in

    $ mkdir /var/tmp/openssl-build
    $ cd /var/tmp/openssl-build
    $ whatever

you cannot simply copy&paste that series of commands, while with

    mkdir /var/tmp/openssl-build
    cd /var/tmp/openssl-build
    whatever

you can.

And I suppose there are just a few cases where, leaving it out, command input and output could be confused.
In such cases we could, e.g., insert a blank line to help understanding this distinction.

Moreover, the $ symbol is not known to Windows users, where typically a > is used as prompt symbol.

@richsalz
Copy link
Contributor

richsalz commented Jun 24, 2020

Some examples intermix command and output and the "$" prompt is used to mark off those.

I notice that "!" is used as a comment char for the OpenVMS line and I'm not sure it should get special treatment.

@DDvO
Copy link
Contributor

DDvO commented Jun 24, 2020

Is there a way in MarkDown to render those $ in a way that they are not copied when selecting and copying text including them?
In HTML for instance there are some tricks for making text non-selectable.

@richsalz
Copy link
Contributor

richsalz commented Jun 24, 2020

There is no way to make something non-selectable in markdown.

@richsalz
Copy link
Contributor

richsalz commented Jun 24, 2020

(FWIW, my prompt is a semicolon

@DDvO
Copy link
Contributor

DDvO commented Jun 24, 2020

There is no way to make something non-selectable in markdown.

I see :-( but thanks for the info!

mspncp
mspncp approved these changes Jun 24, 2020
View reviewed changes
Copy link
Contributor

@mspncp mspncp left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Many of the code examples are really just examples and are not be meant to be copy&pasted literally. Some people might be tempted to blindly copy&paste large code blocks without checking the code. So having to edit the lines gives them a little extra time to switch on their brain. Like in the following code block:

    $ make clean                                     # Unix
    $ mms clean                                      ! (or mmk) OpenVMS
    $ nmake clean                                    # Windows

P.S: programmers usually tend to have smart editors which are capable of block editing. ;-)

@DDvO DDvO mentioned this pull request Jun 25, 2020
Closed
@openssl-machine
Copy link
Collaborator

openssl-machine commented Jun 25, 2020

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

openssl-machine pushed a commit that referenced this pull request Jun 27, 2020
@levitte
INSTALL.md: Restore $ as command prompt indicator
270540f 
We have a notational convention in INSTALL.md, which says this among
others:

> Any line starting with a dollar sign is a command line.
>
>     $ command
>
> The dollar sign indicates the shell prompt and is not to be entered as
> part of the command.

That notation exists to make it clear what is a command line and
what's output from that command line.

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Matthias St. Pierre <[email protected]>
(Merged from #12257)
@levitte
Copy link
Member Author

levitte commented Jun 27, 2020

Merged

270540f INSTALL.md: Restore $ as command prompt indicator

@levitte levitte closed this Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattcaswell mattcaswell mattcaswell approved these changes

+1 more reviewer

@mspncp mspncp mspncp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@levitte @richsalz @DDvO @openssl-machine @mattcaswell @mspncp