documentation: handle non-existing html pages and document 'git version' #1038
Conversation
Welcome to GitGitGadgetHi @rimrul, 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. 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:
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 patchesBefore 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 Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a 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 curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo 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): To send a new iteration, just add another PR comment with the contents: 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, |
rimrul
force-pushed
the
doc-version
branch
2 times, most recently
from
bb9f4b7
to
d3635cb
Compare
rimrul
changed the title
|
/allow |
|
User rimrul is now allowed to use GitGitGadget. WARNING: rimrul has no public email address set on GitHub |
|
/preview |
|
Error: Could not determine public email of rimrul |
|
/preview |
|
Error: Could not determine public email of rimrul |
|
/preview |
|
Preview email sent as pull.1038.git.1631530375.gitgitgadget@gmail.com |
|
/submit |
|
Submitted as pull.1038.git.1631531218.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -0,0 +1,35 @@ | |||
| git-version(1) | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Mon, Sep 13 2021, Matthias Aßhauer via GitGitGadget wrote:
> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
>
> While 'git version' is probably the least complex git command,
> it is a non-experimental user-facing builtin command. As such
> it should have a help page.
This looks good
> Signed-off-by: Matthias Aßhauer <mha1993@live.de>
> ---
> Documentation/git-version.txt | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/git-version.txt
>
> diff --git a/Documentation/git-version.txt b/Documentation/git-version.txt
> new file mode 100644
> index 00000000000..c7d6b496c8d
> --- /dev/null
> +++ b/Documentation/git-version.txt
> @@ -0,0 +1,35 @@
> +git-version(1)
> +==============
> +
> +NAME
> +----
> +git-version - Display version information about Git
> +
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git version' [--build-options]
>
> +
> +DESCRIPTION
> +-----------
> +
> +With no options given, the version of 'git' is printed
> +on the standard output.
Good
> +
> +If the option `--build-options` is given, information about how git was built is
> +printed on the standard output in addition to the version number.
Let's just cover this in the OPTIONS section you added...
> +Note that `git --version` is identical to `git version` because the
> +former is internally converted into the latter.
Probably better to just have a new section:
SEE ALSO
--------
linkgit:git[1]'s `--version` option, which dispatches to this command.
> +OPTIONS
> +-------
> +--build-options::
> + Prints out additional information about how git was built for diagnostic
> + purposes.
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
It would also be good to update git.txt, which now says:
Prints the Git suite version that the git program came from
To say e.g. "Dispatches to linkgit:git-version[1], prints the git
program version".
Or something like that, i.e. to cross-link the two.
There was a problem hiding this comment.
On the Git mailing list, Matthias Aßhauer wrote (reply to this):
On Mon, 13 Sep 2021, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Sep 13 2021, Matthias Aßhauer via GitGitGadget wrote:
>
>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
>>
>> While 'git version' is probably the least complex git command,
>> it is a non-experimental user-facing builtin command. As such
>> it should have a help page.
>
> This looks good
>
>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>
>> ---
>> Documentation/git-version.txt | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>> create mode 100644 Documentation/git-version.txt
>>
>> diff --git a/Documentation/git-version.txt b/Documentation/git-version.txt
>> new file mode 100644
>> index 00000000000..c7d6b496c8d
>> --- /dev/null
>> +++ b/Documentation/git-version.txt
>> @@ -0,0 +1,35 @@
>> +git-version(1)
>> +==============
>> +
>> +NAME
>> +----
>> +git-version - Display version information about Git
>> +
>> +
>> +SYNOPSIS
>> +--------
>> +[verse]
>> +'git version' [--build-options]
>>
>> +
>> +DESCRIPTION
>> +-----------
>> +
>> +With no options given, the version of 'git' is printed
>> +on the standard output.
>
> Good
>
>> +
>> +If the option `--build-options` is given, information about how git was built is
>> +printed on the standard output in addition to the version number.
>
> Let's just cover this in the OPTIONS section you added...
Ok, I can absolutely do that.
>
>> +Note that `git --version` is identical to `git version` because the
>> +former is internally converted into the latter.
>
> Probably better to just have a new section:
>
> SEE ALSO
> --------
>
> linkgit:git[1]'s `--version` option, which dispatches to this command.
>
>
I've closely based this on git-help.txt, since `--help` and `--version`
both are options that get internally converted to the corresponding command.
>> +OPTIONS
>> +-------
>> +--build-options::
>> + Prints out additional information about how git was built for diagnostic
>> + purposes.
>> +
>> +GIT
>> +---
>> +Part of the linkgit:git[1] suite
>
>
> It would also be good to update git.txt, which now says:
>
> Prints the Git suite version that the git program came from
>
> To say e.g. "Dispatches to linkgit:git-version[1], prints the git
> program version".
>
> Or something like that, i.e. to cross-link the two.
That makes sense. Should we do the same for '--help'?
Best regards
Matthias
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> +Note that `git --version` is identical to `git version` because the
>> +former is internally converted into the latter.
>
> Probably better to just have a new section:
>
> SEE ALSO
> --------
>
> linkgit:git[1]'s `--version` option, which dispatches to this command.
Hmph, I am not sure if this is a good move.
If we are not giving any more information than what the reader has
already learned from this page, other than "git --version" does the
same thing, we probably do not want to do this. By seeing also that
other page, the user will not learn anything new about "git version".
If a related "git --version-something-else" is described over there
and may fill the need the reader had when visiting this page, that
is a different story, but I do not think it is the case.
>> +OPTIONS
>> +-------
>> +--build-options::
>> + Prints out additional information about how git was built for diagnostic
>> + purposes.
>> +
>> +GIT
>> +---
>> +Part of the linkgit:git[1] suite
>
>
> It would also be good to update git.txt, which now says:
>
> Prints the Git suite version that the git program came from
>
> To say e.g. "Dispatches to linkgit:git-version[1], prints the git
> program version".
This one may be a good idea, I think.
"git --version --build-options" also works and we do not want to
clutter git[1] with descriptions on suboptions of "git version".
If we are not doing so for the "--help" option in git[1], we should
do so as well.
Thanks.
|
User |
| @@ -467,11 +467,18 @@ static void get_html_page_path(struct strbuf *page_path, const char *page) | |||
| if (!html_path) | |||
There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Mon, Sep 13, 2021 at 7:07 AM Matthias Aßhauer via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> We already check that git.html exists, regardless of the page the user wants
> to open. Additionally checking wether the requested page exists gives us a
s/wether/whether/
> smoother user experience when it doesn't.
> When calling a git command and there is an error, most users reasonably expect
> git to produce an error message on the standard error stream, but in this case
> we pass the filepath to git web--browse wich passes it on to a browser (or a
s/wich/which/
> helper programm like xdg-open or start that should in turn open a browser)
s/programm/program/
> without any error and many GUI based browsers or helpers won't output such a
> message onto the standard error stream.
>
> Especialy the helper programs tend to show the corresponding error message in
s/Especialy/Especially/
> a message box and wait for user input before exiting. This leaves users in
> interactive console sessions without an error message in their console,
> without a console prompt and without the help page they expected.
>
> The performance cost of the additional stat should be negliggible compared to
s/negliggible/negligible/
> the two or more pocesses that we spawn after the checks.
s/pocesses/processes/
> Signed-off-by: Matthias Aßhauer <mha1993@live.de>
There was a problem hiding this comment.
On the Git mailing list, Matthias Aßhauer wrote (reply to this):
On Mon, 13 Sep 2021, Eric Sunshine wrote:
> On Mon, Sep 13, 2021 at 7:07 AM Matthias Aßhauer via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> We already check that git.html exists, regardless of the page the user wants
>> to open. Additionally checking wether the requested page exists gives us a
>
> s/wether/whether/
>
>> smoother user experience when it doesn't.
>
>> When calling a git command and there is an error, most users reasonably expect
>> git to produce an error message on the standard error stream, but in this case
>> we pass the filepath to git web--browse wich passes it on to a browser (or a
>
> s/wich/which/
>
>> helper programm like xdg-open or start that should in turn open a browser)
>
> s/programm/program/
>
>> without any error and many GUI based browsers or helpers won't output such a
>> message onto the standard error stream.
>>
>> Especialy the helper programs tend to show the corresponding error message in
>
> s/Especialy/Especially/
>
>> a message box and wait for user input before exiting. This leaves users in
>> interactive console sessions without an error message in their console,
>> without a console prompt and without the help page they expected.
>>
>> The performance cost of the additional stat should be negliggible compared to
>
> s/negliggible/negligible/
>
>> the two or more pocesses that we spawn after the checks.
>
> s/pocesses/processes/
>
>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>
>
Thank you for pointing out this embarrassing amount of typos.
I've fixed them for the next version.
Best regards
Matthias
There was a problem hiding this comment.
I think GitGitGadget doesn't like that I marked this as resolved after addressing Erics feedback. There is a mail from Junio on the mailing list regarding this patch that doesn't show up on GitHub.
There was a problem hiding this comment.
That shouldn't matter... maybe the Azure Pipelines got stuck...
There was a problem hiding this comment.
hrm. does not look like it: https://dev.azure.com/gitgitgadget/git/_build?definitionId=5
sorry, I have no idea what is going on... and I'm on a phone right now, can't dig further...
There was a problem hiding this comment.
No problem. I just wanted to mention it for the record. I've read Junios feedback and everyone else who's interested can read it via the link above.
There was a problem hiding this comment.
Hrm. Looks as if the Pipeline encountered a strange error:
HttpError: Validation Failed: {"resource":"PullRequestReviewComment","code":"custom","field":"pull_request_review_thread.end_commit_oid","message":"pull_request_review_thread.end_commit_oid is not part of the pull request"}: skipping
Quite honestly, I have no idea what to make of this.
There was a problem hiding this comment.
Oh. I think I can (kind of) provide an insight on this. Junio commented on Patch 1/2 as sent to the mailing list. But based on the feedback I received from Eric, I had already pushed a new version of that patch with the typos in the log message fixed. Maybe it got confused by that?
|
User |
|
User |
rimrul
force-pushed
the
doc-version
branch
2 times, most recently
from
821e368
to
1c0f54d
Compare
|
/preview |
|
Preview email sent as pull.1038.v2.git.1631623752.gitgitgadget@gmail.com |
We check that git.html exists, regardless of the page the user wants to open. Checking whether the requested page exists instead gives us a smoother user experience in two use cases: 1) The requested page doesn't exist When calling a git command and there is an error, most users reasonably expect git to produce an error message on the standard error stream, but in this case we pass the filepath to git web--browse which passes it on to a browser (or a helper program like xdg-open or start that should in turn open a browser) without any error and many GUI based browsers or helpers won't output such a message onto the standard error stream. Especially the helper programs tend to show the corresponding error message in a message box and wait for user input before exiting. This leaves users in interactive console sessions without an error message in their console, without a console prompt and without the help page they expected. 2) git.html is missing for some reason, but the user asked for some other page We currently refuse to show any local html help page when we can't find git.html. Even if the requested help page exists. If we check for the requested page instead, we can show the user all available pages and only error out on those that don't exist. Signed-off-by: Matthias Aßhauer <mha1993@live.de>
While 'git version' is probably the least complex git command, it is a non-experimental user-facing builtin command. As such it should have a help page. Both `git help` and `git version` can be called as options (`--help`/`--version`) that internally get converted to the corresponding command. Add a small paragraph to Documentation/git.txt describing how these two options interact with each other and link to this help page for the sub-options that `--version` can take. Well, currently there is only one sub-option, but that could potentially increase in future versions of Git. Signed-off-by: Matthias Aßhauer <mha1993@live.de>
|
/cc "Junio C Hamano" gitster@pobox.com |
|
/preview |
|
Preview email sent as pull.1038.v2.git.1631625903.gitgitgadget@gmail.com |
|
/submit |
|
Submitted as pull.1038.v2.git.1631626038.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This branch is now known as |
|
This patch series was integrated into seen via git@aa33601. |
That's patch 2. Patch 1 is a separate integration branch |
|
This patch series was integrated into seen via git@e2fc239. |
|
This patch series was integrated into seen via git@f5b2a15. |
|
This patch series was integrated into next via git@54a6a3f. |
|
There was a status update in the "New Topics" section about the branch Doc update. Will merge to 'master'. |
|
The other topic also got a mention in What's cooking: |
|
This patch series was integrated into seen via git@20cb59f. |
|
There was a status update in the "Cooking" section about the branch Doc update. Will merge to 'master'. |
|
This patch series was integrated into seen via git@188da7d. |
|
This patch series was integrated into next via git@188da7d. |
|
This patch series was integrated into master via git@188da7d. |
|
Closed via 188da7d. |
| @@ -0,0 +1,28 @@ | |||
| git-version(1) | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Tue, Sep 14 2021, Matthias Aßhauer via GitGitGadget wrote:
> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
>
> While 'git version' is probably the least complex git command,
> it is a non-experimental user-facing builtin command. As such
> it should have a help page.
>
> Both `git help` and `git version` can be called as options
> (`--help`/`--version`) that internally get converted to the
> corresponding command. Add a small paragraph to
> Documentation/git.txt describing how these two options
> interact with each other and link to this help page for the
> sub-options that `--version` can take. Well, currently there
> is only one sub-option, but that could potentially increase
> in future versions of Git.
>
> Signed-off-by: Matthias Aßhauer <mha1993@live.de>
> ---
> Documentation/git-version.txt | 28 ++++++++++++++++++++++++++++
> Documentation/git.txt | 4 ++++
> 2 files changed, 32 insertions(+)
> create mode 100644 Documentation/git-version.txt
>
> diff --git a/Documentation/git-version.txt b/Documentation/git-version.txt
> new file mode 100644
> index 00000000000..80fa7754a6d
> --- /dev/null
> +++ b/Documentation/git-version.txt
> @@ -0,0 +1,28 @@
> +git-version(1)
> +==============
> +
> +NAME
> +----
> +git-version - Display version information about Git
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git version' [--build-options]
> +
> +DESCRIPTION
> +-----------
> +With no options given, the version of 'git' is printed on the standard output.
> +
> +Note that `git --version` is identical to `git version` because the
> +former is internally converted into the latter.
> +
> +OPTIONS
> +-------
> +--build-options::
> + Include additional information about how git was built for diagnostic
> + purposes.
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 6dd241ef838..95fe6f31b4f 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -41,6 +41,10 @@ OPTIONS
> -------
> --version::
> Prints the Git suite version that the 'git' program came from.
> ++
> +This option is internaly converted to `git version ...` and accepts
> +the same options as the linkgit:git-version[1] command. If `--help` is
> +also given, it takes precedence over `--version`.
>
> --help::
> Prints the synopsis and a list of the most commonly used
I didn't notice until after it hit master that this caused a regression
in "make check-docs":
$ make -s check-docs
removed but documented: git-version
The "fix" is rather easy, i.e. adding "git-version" to the whitelist.
But I wondered about $subject, i.e. we want to run the "lint" part, but
do we really need something reminding us that there isn't a mapping
between Documentation/*.txt and *.o files present at the top-level?
That whole part seems to have been some "reminder to document" addition
in 8c989ec5288 (Makefile: $(MAKE) check-docs, 2006-04-13).
If we're going to keep it in pretty much its current form then the CI
integration added in b98712b9aa9 (travis-ci: build documentation,
2016-05-04) seems rather useless when it comes to this, i.e. we should
either adjust it to exit non-zero, or check if we've got output under
"make -s" and fail the check then.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> I didn't notice until after it hit master that this caused a regression
> in "make check-docs":
>
> $ make -s check-docs
> removed but documented: git-version
>
> The "fix" is rather easy, i.e. adding "git-version" to the whitelist.
>
> But I wondered about $subject, i.e. we want to run the "lint" part, but
> do we really need something reminding us that there isn't a mapping
> between Documentation/*.txt and *.o files present at the top-level?
There were multiple things check-docs wanted to catch originally.
- commands not referred to from the main page
- a new command added without documentation
- an old command removed while leaving documentation
It may be that we no longer remove commands, so the last check may
be less useful.
> If we're going to keep it in pretty much its current form then the CI
> integration added in b98712b9aa9 (travis-ci: build documentation,
> 2016-05-04) seems rather useless when it comes to this, i.e. we should
> either adjust it to exit non-zero,...
Yes, that is a good thing to do.
Thanks.
These two patches are grouped as one patch series, because they arose from the
same Git for Windows issue [1], but they can be reviewed or applied independent
from one another.
One interesting oddity I found while preparing V2:
git --version --helpgets converted togit version --helpwhich then callsgit help version,but
git --help --versiongets converted togit help --versionandgit helpdoesn't know what to do with--version.[1] git-for-windows#3308
Changes since V1:
git.html--build-optionsfrom theDESCRIPTIONsectionDocumentation/git.txtthat links to the newgit versionpage (like we already do forgit help)cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: Eric Sunshine sunshine@sunshineco.com
cc: Matthias Aßhauer mha1993@live.de
cc: Junio C Hamano gitster@pobox.com