doc: apply new format to git-branch man page #1880
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jnavila
force-pushed
the
doc_git_branch
branch
5 times, most recently
from
0eb4dbf to
5d2afbc
Compare
|
There are issues in commit 4386d71: |
|
/submit |
|
Submitted as [email protected] 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}
| @@ -13,8 +13,8 @@ print_config_list () { | |||
| cat <<EOF | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
>
> With the modern formatting of the manpages, the options and commands are now
> backticked in their definition lists. This patch updates the generation of
> the completion list to take into account this new format.
>
> Signed-off-by: Jean-Noël Avila <[email protected]>
> ---
> generate-configlist.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> index dffdaada8b5..802178daad4 100755
> --- a/generate-configlist.sh
> +++ b/generate-configlist.sh
> @@ -13,8 +13,8 @@ print_config_list () {
> cat <<EOF
> static const char *config_name_list[] = {
> EOF
> - grep -h '^[a-zA-Z].*\..*::$' "$SOURCE_DIR"/Documentation/*config.adoc "$SOURCE_DIR"/Documentation/config/*.adoc |
> - sed '/deprecated/d; s/::$//; s/, */\n/g' |
> + grep -h '^`\?[a-zA-Z].*\..*`\?::$' "$SOURCE_DIR"/Documentation/*config.adoc "$SOURCE_DIR"/Documentation/config/*.adoc |
Using \? in BRE as a short-hand for \{0,1\} (or trying to use any
single regexp magic in BRE by adding a backslash when that magic is
only available in ERE) is a GNUism, isn't it?
We probably should rewrite the thing as ERE, perhaps like
grep -E -h '^`?[a-zA-Z].*\..*`?::$' ...
Also, if we can avoid piping grep into sed or awk, we should do so,
when it does not lose readability. Perhaps we can do something like
sed -E -n -e '/... that pattern .../{
/deprecated/d;
s/::$//;
...
}' "$SOURCE_DIR"/Documentation/*config.adoc ... |
sort |
...
in this case?
> + sed '/deprecated/d; s/::$//; s/`//g; s/, */\n/g' |
> sort |
> sed 's/^.*$/ "&",/'
> cat <<EOFThere was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Monday, 17 March 2025 21:52:13 CET Junio C Hamano wrote:
> "Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
> >
> > With the modern formatting of the manpages, the options and commands are
now
> > backticked in their definition lists. This patch updates the generation of
> > the completion list to take into account this new format.
> >
> > Signed-off-by: Jean-Noël Avila <[email protected]>
> > ---
> >
> > generate-configlist.sh | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/generate-configlist.sh b/generate-configlist.sh
> > index dffdaada8b5..802178daad4 100755
> > --- a/generate-configlist.sh
> > +++ b/generate-configlist.sh
> > @@ -13,8 +13,8 @@ print_config_list () {
> >
> > cat <<EOF
> >
> > static const char *config_name_list[] = {
> > EOF
> >
> > - grep -h '^[a-zA-Z].*\..*::$' "$SOURCE_DIR"/Documentation/
*config.adoc
> > "$SOURCE_DIR"/Documentation/config/*.adoc | - sed '/deprecated/d; s/::$//;
> > s/, */\n/g' |
> > + grep -h '^`\?[a-zA-Z].*\..*`\?::$'
> > "$SOURCE_DIR"/Documentation/*config.adoc
> > "$SOURCE_DIR"/Documentation/config/*.adoc |
> Using \? in BRE as a short-hand for \{0,1\} (or trying to use any
> single regexp magic in BRE by adding a backslash when that magic is
> only available in ERE) is a GNUism, isn't it?
>
> We probably should rewrite the thing as ERE, perhaps like
>
> grep -E -h '^`?[a-zA-Z].*\..*`?::$' ...
>
> Also, if we can avoid piping grep into sed or awk, we should do so,
> when it does not lose readability. Perhaps we can do something like
>
> sed -E -n -e '/... that pattern .../{
> /deprecated/d;
> s/::$//;
> ...
> }' "$SOURCE_DIR"/Documentation/*config.adoc ... |
> sort |
> ...
>
> in this case?
For the GNUism, the tests on MacOS and Windows by gitgitgadget passed. But I
get your point and will reroll.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jean-Noël AVILA <[email protected]> writes:
> For the GNUism, the tests on MacOS and Windows by gitgitgadget passed. But I
> get your point and will reroll.
Is there a good test in our test suite that validates the output of
this script? I had an impression that even if the regexp match by
this grep were a bit off, the only end-user visible effect of such a
breakage is that some entries from config_name_list[] may be missing
when "git help --config-for-completion" is called, but I do not
think of any sensible way to notice that some entries are missing or
extra entries exist in the output. So unless the regexp is broken
so badly that makes the resulting config-list.h syntactically
incorrect, it is unlikely that our test suite would catch anything,
I suspect.
If I deliberately break the regexp (this is before your patch), it
does not seem to break t0012 (which uses --config-for-completion).
generate-configlist.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git c/generate-configlist.sh w/generate-configlist.sh
index dffdaada8b..a6eb9739ea 100755
--- c/generate-configlist.sh
+++ w/generate-configlist.sh
@@ -13,7 +13,7 @@ print_config_list () {
cat <<EOF
static const char *config_name_list[] = {
EOF
- grep -h '^[a-zA-Z].*\..*::$' "$SOURCE_DIR"/Documentation/*config.adoc "$SOURCE_DIR"/Documentation/config/*.adoc |
+ grep -h '^[a-uA-Z].*\..*::$' "$SOURCE_DIR"/Documentation/*config.adoc "$SOURCE_DIR"/Documentation/config/*.adoc |
sed '/deprecated/d; s/::$//; s/, */\n/g' |
sort |
sed 's/^.*$/ "&",/'
There was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Tuesday, 18 March 2025 05:16:32 CET Junio C Hamano wrote:
> Jean-Noël AVILA <[email protected]> writes:
> > For the GNUism, the tests on MacOS and Windows by gitgitgadget passed. But
I
> > get your point and will reroll.
>
> Is there a good test in our test suite that validates the output of
> this script? I had an impression that even if the regexp match by
> this grep were a bit off, the only end-user visible effect of such a
> breakage is that some entries from config_name_list[] may be missing
> when "git help --config-for-completion" is called, but I do not
> think of any sensible way to notice that some entries are missing or
> extra entries exist in the output. So unless the regexp is broken
> so badly that makes the resulting config-list.h syntactically
> incorrect, it is unlikely that our test suite would catch anything,
> I suspect.
>
> If I deliberately break the regexp (this is before your patch), it
> does not seem to break t0012 (which uses --config-for-completion).
>
I noticed the bug when working with git-branch's doc, because it broke t9902
which specifically tests completion for 'git config get br' and could no longer
find the 'branch.' So the tests have eventually found the regression when the
formatting went a little more widespread.
As for a way to validate the content of the script, I can only think of
committing the config-list.h and checking when the contents diverge.
| @@ -1,41 +1,42 @@ | |||
| branch.autoSetupMerge:: | |||
| Tells 'git branch', 'git switch' and 'git checkout' to set up new branches | |||
| `branch.autoSetupMerge`:: | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
>
> - Switch the synopsis to a synopsis block which automatically
> formats placeholders in italics and keywords in monospace
> - Use _<placeholder>_ instead of <placeholder> in the description
> - Use `backticks` for keywords and more complex option
> descriptions. The new rendering engine applies synopsis rules to
> these spans.
And the one effective reformatting in this patch that is not covered
by the list is that the possible values for some variables, that
were mentioned in the description prose, are now made into
enumerated list.
Nicely done.
|
This patch series was integrated into seen via git@30423c7. |
With the modern formatting of the manpages, the options and commands are now backticked in their definition lists. This patch updates the generation of the completion list to take into account this new format. The script `generate-configlist.sh` is updated to get rid of extraneous commands and fit everything in a single sed script. Signed-off-by: Jean-Noël Avila <[email protected]>
- Switch the synopsis to a synopsis block which automatically formats placeholders in italics and keywords in monospace - Use _<placeholder>_ instead of <placeholder> in the description - Use `backticks` for keywords and more complex option descriptions. The new rendering engine applies synopsis rules to these spans. Possible values for some variables, that were mentioned in the description prose, are now made into enumerated list. Signed-off-by: Jean-Noël Avila <[email protected]>
|
This branch is now known as |
|
This patch series was integrated into seen via git@c22de74. |
|
This patch series was integrated into seen via git@643f7a6. |
|
There was a status update in the "New Topics" section about the branch Expecting a reroll to lose GNUism. cf. <2773494.mvXUDI8C0e@cayenne> source: <[email protected]> |
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@6b27c88. |
|
There was a status update in the "Cooking" section about the branch Doc mark-up updates. Will merge to 'next'. source: <[email protected]> |
|
This patch series was integrated into seen via git@d8c1d77. |
|
This patch series was integrated into next via git@ba6e1c7. |
|
This patch series was integrated into seen via git@e9745c0. |
|
There was a status update in the "Cooking" section about the branch Doc mark-up updates. Will merge to 'master'. source: <[email protected]> |
|
This patch series was integrated into seen via git@0baa915. |
|
This patch series was integrated into seen via git@89054f6. |
|
There was a status update in the "Cooking" section about the branch Doc mark-up updates. Will merge to 'master'. source: <[email protected]> |
|
This patch series was integrated into seen via git@6a9e1c3. |
|
This patch series was integrated into master via git@6a9e1c3. |
|
This patch series was integrated into next via git@6a9e1c3. |
|
Closed via 6a9e1c3. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Working on git-branch's doc uncovered a bug in the completion logic that did not take into account the new formatting of options. Apart from that, the changes are quite standard now.
Changes since V1: