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

INSTALL: use '$', not '#' for user-run command prompt #1241

Closed
wants to merge 1 commit into from
Closed

INSTALL: use '$', not '#' for user-run command prompt #1241

wants to merge 1 commit into from

Conversation

sunshaoce
Copy link

@sunshaoce sunshaoce commented May 24, 2022

The user prompt should be $ instead of #.

cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: Philippe Blain levraiphilippeblain@gmail.com
cc: Christian Couder christian.couder@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

Welcome to GitGitGadget

Hi @sunshaoce, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that your Pull Request has a good description, as it will be used as cover letter. You can CC potential reviewers by adding a footer to the PR description with the following syntax:

CC: Revi Ewer <revi.ewer@example.com>, Ill Takalook <ill.takalook@example.net>

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "revisions:" to state which subsystem the change is about, and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the Libera Chat IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this. Note that any reviewers CC'd via the list in the PR description will not actually be sent emails.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Libera Chat. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

There are issues in commit 900f38d:
Fix wrong info in INSTALL``
Commit not signed off

@sunshaoce
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

Error: User sunshaoce is not yet permitted to use GitGitGadget

@sunshaoce
Copy link
Author

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

Error: User sunshaoce is not yet permitted to use GitGitGadget

@dscho
Copy link
Member

dscho commented May 24, 2022

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

User sunshaoce is now allowed to use GitGitGadget.

WARNING: sunshaoce has no public email address set on GitHub;
GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use.

@sunshaoce
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

Submitted as pull.1241.git.1653424998869.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1241/sunshaoce/install-v1

To fetch this version to local tag pr-1241/sunshaoce/install-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1241/sunshaoce/install-v1

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):

On Tue, May 24 2022, Shao-Ce SUN via GitGitGadget wrote:

> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>
> The user prompt should be `$` instead of `#`.
>
> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
> ---
>     Fix wrong info in INSTALL
>     
>     The user prompt should be $ instead of #.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1241%2Fsunshaoce%2Finstall-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1241/sunshaoce/install-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1241
>
>  INSTALL | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index 4140a3f5c8b..7bb3f48311d 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>  will install the git programs in your own ~/bin/ directory.  If you want
>  to do a global install, you can do
>  
> -	$ make prefix=/usr all doc info ;# as yourself
> -	# make prefix=/usr install install-doc install-html install-info ;# as root
> +	$ make prefix=/usr all doc info ; $ as yourself
> +	# make prefix=/usr install install-doc install-html install-info ; # as root
>  
>  (or prefix=/usr/local, of course).  Just like any program suite
>  that uses $prefix, the built results have some paths encoded,
> @@ -20,10 +20,10 @@ config.mak file.
>  Alternatively you can use autoconf generated ./configure script to
>  set up install paths (via config.mak.autogen), so you can write instead
>  
> -	$ make configure ;# as yourself
> -	$ ./configure --prefix=/usr ;# as yourself
> -	$ make all doc ;# as yourself
> -	# make install install-doc install-html;# as root
> +	$ make configure ; $ as yourself
> +	$ ./configure --prefix=/usr ; $ as yourself
> +	$ make all doc ; $ as yourself
> +	# make install install-doc install-html; # as root
>  
>  If you're willing to trade off (much) longer build time for a later
>  faster git you can also do a profile feedback build with
>
> base-commit: 7a3eb286977746bc09a5de7682df0e5a7085e17c

This looks good to me, FWIW I dug into this slightly and didn't know
that POSIX had this to say about it:

        This variable is used for interactive prompts. Historically, the
        "superuser" has had a prompt of '#'. Since privileges are not
        required to be monolithic, it is difficult to define which
        privileges should cause the alternate prompt. However, a
        sufficiently powerful user should be reminded of that power by
        having an alternate prompt.

See https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html#tag_23_02_05_03

The one suggestion I have here is that the $subject should be clearer, e.g.:

    INSTALL: use '#', not '$' for root-run command prompt

Which in this case would both be better in --oneline output, and be
enough to get rid of the commit message body entirely (unless it wished
to say something more on the subject).

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

User Ævar Arnfjörð Bjarmason <avarab@gmail.com> has been added to the cc: list.

@sunshaoce sunshaoce changed the title Fix wrong info in INSTALL INSTALL: use '$', not '#' for user-run command prompt May 24, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

There are issues in commit 900f38d:
Fix wrong info in INSTALL``
Commit not signed off

@sunshaoce
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

There are issues in commit 900f38d:
Fix wrong info in INSTALL``
Commit not signed off

Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

On the Git mailing list, Philippe Blain wrote (reply to this):

Hi Shao-Ce,

Le 2022-05-24 à 17:39, Ævar Arnfjörð Bjarmason a écrit :
> 
> On Tue, May 24 2022, Shao-Ce SUN via GitGitGadget wrote:
> 
>> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>>
>> The user prompt should be `$` instead of `#`.
>>
>> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>> ---
>>     Fix wrong info in INSTALL
>>     
>>     The user prompt should be $ instead of #.
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1241%2Fsunshaoce%2Finstall-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1241/sunshaoce/install-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1241
>>
>>  INSTALL | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/INSTALL b/INSTALL
>> index 4140a3f5c8b..7bb3f48311d 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>>  will install the git programs in your own ~/bin/ directory.  If you want
>>  to do a global install, you can do
>>  
>> -	$ make prefix=/usr all doc info ;# as yourself
>> -	# make prefix=/usr install install-doc install-html install-info ;# as root
>> +	$ make prefix=/usr all doc info ; $ as yourself
>> +	# make prefix=/usr install install-doc install-html install-info ; # as root

The prompt for the first invocation is already '$',
what you are changing in the first line is the '# as yourself' comment
which is a shell comment and thus uses '#'. This allows 
the whole line to be pasted as-is.

The second line is supposed to be ran as root and already has '#' as
prompt, and also has '# as root' as a comment, which you simply
prefix with a space.

I don't think there is anything to "fix" here...

>>  
>>  (or prefix=/usr/local, of course).  Just like any program suite
>>  that uses $prefix, the built results have some paths encoded,
>> @@ -20,10 +20,10 @@ config.mak file.
>>  Alternatively you can use autoconf generated ./configure script to
>>  set up install paths (via config.mak.autogen), so you can write instead
>>  
>> -	$ make configure ;# as yourself
>> -	$ ./configure --prefix=/usr ;# as yourself
>> -	$ make all doc ;# as yourself
>> -	# make install install-doc install-html;# as root
>> +	$ make configure ; $ as yourself
>> +	$ ./configure --prefix=/usr ; $ as yourself
>> +	$ make all doc ; $ as yourself
>> +	# make install install-doc install-html; # as root

Same here.

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

User Philippe Blain <levraiphilippeblain@gmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented May 24, 2022

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Shao-Ce SUN via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -	$ make prefix=/usr all doc info ;# as yourself
> -	# make prefix=/usr install install-doc install-html install-info ;# as root
> +	$ make prefix=/usr all doc info ; $ as yourself

This and all the changes in the patch is wrong, but it is not
entirely your fault.  You need to know the shell syntax.

The semicolon in ";#" is an end of one command (i.e. "make"), and
the hash is "start of comment--ignore the rest of the line.

If you replace # with $, then "$ as yourself" would be fed to the
shell because $ is not a comment introducer.  The command line no
longer can be copied and pasted, starting from "make" to the end of
the line.

Thanks.

@gitgitgadget
Copy link

gitgitgadget bot commented May 25, 2022

On the Git mailing list, Christian Couder wrote (reply to this):

On Wed, May 25, 2022 at 6:35 AM Shao-Ce SUN via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>
> The user prompt should be `$` instead of `#`.

On the following lines:

$ make prefix=/usr all doc info ;# as yourself
# make prefix=/usr install install-doc install-html install-info ;# as root

the '$' or '#' before "make" is indeed a prompt, but the '#' before
"as" starts a single line comment. So there is no need to change it
into a '$'.

See for example:

https://tldp.org/LDP/abs/html/abs-guide.html#SPECIAL-CHARS

One issue with the above lines could be that there is no space
character before the '#' that starts the comments. The ';' is a
command separator character though, and I think that when a new
command starts with '#' it's considered a comment.

So we could perhaps get rid of the ';', but on the other hand ";#"
should make it extra clear that the command before it ended and that
the rest of the line is a comment.

> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
> ---
>     Fix wrong info in INSTALL

Instead of "info" you could say something more specific like "prompt character".

Also we start commit subjects with the area of the code, so maybe
"INSTALL: fix wrong prompt character".

> diff --git a/INSTALL b/INSTALL
> index 4140a3f5c8b..7bb3f48311d 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>  will install the git programs in your own ~/bin/ directory.  If you want
>  to do a global install, you can do
>
> -       $ make prefix=/usr all doc info ;# as yourself
> -       # make prefix=/usr install install-doc install-html install-info ;# as root
> +       $ make prefix=/usr all doc info ; $ as yourself

Here a '#' is changed into a '$' but also a space character is
inserted before the '$' without any explanation.

> +       # make prefix=/usr install install-doc install-html install-info ; # as root

Here only a space character is inserted before the second '#' without
any explanation.

@gitgitgadget
Copy link

gitgitgadget bot commented May 25, 2022

User Christian Couder <christian.couder@gmail.com> has been added to the cc: list.

@sunshaoce
Copy link
Author

sunshaoce commented May 25, 2022 via email

@sunshaoce sunshaoce closed this May 25, 2022
@dscho
Copy link
Member

dscho commented May 25, 2022

Thanks everyone! I’ll close the PR.

@sunshaoce Could you please reply via mail to https://lore.kernel.org/git/pull.1241.git.1653424998869.gitgitgadget@gmail.com as indicated in https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis, as most Git developers only look at the Git mailing list? Thank you.

@gitgitgadget
Copy link

gitgitgadget bot commented May 25, 2022

On the Git mailing list, Junio C Hamano wrote (reply to this):

Christian Couder <christian.couder@gmail.com> writes:

> So we could perhaps get rid of the ';', but on the other hand ";#"
> should make it extra clear that the command before it ended and that
> the rest of the line is a comment.

True.  In modern shells, ";" in the ";#" sequence may not be
necessary, but consistently writing ";#" as if it were a single
token would probably be a good idea in the documentation.

Of course, if we can declare that the usefulness of the monospace
plain text document does not matter, we could do without ";#" and
typeset the comment (and the prompt) in different font and/or color.
It would make it easier to see which part of the line is what is to
be typed verbatim.

Thanks.

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

Successfully merging this pull request may close these issues.

None yet

2 participants