Message ID | pull.1765.git.1721496853517.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 6474da0aa4d411e12fd4962b3e81c6d4e9db6b12 |
Headers | show |
Series | doc: git-clone fix discrepancy between asciidoc and asciidoctor | expand |
"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr> > > Asciidoc.py does not have the concept of generalized roles, whereas > asciidoctor interprets [foo]`blah` as blah with role foo in the > synopsis, making in effect foo disappear in the output. Note that > square brackets not directly followed by an inline markup do not > define a role, which is why we do not have the issue on other parts of > the documentation. This was utterly misleading, at least to me, and it took a few times of reading for me to really understand what you wanted to say. It does not matter that "generalized roles" is not known by AsciiDoc. What really causes the breakage is that a string inside [square brackets] followed by string inside `a pair of back quotes` is interpreted by Asciidoctor as "generalized roles" (whatever it is), even though we do not care at all about such a feature here. How about phrasing it more like so? Writing a string inside [square brackets], immediately followed by a string inside `a pair of back quotes`, causes asciidoctor to eliminate the string inside [square brackets], because it is a syntax to trigger a "generalized role" feature, which we do not care about in the context of the synopsis section here. Work it around by inserting an otherwise no-op {empty} string to forbid asciidoctor from triggering that feature here. AsciiDoc is not affected negatively by this additional empty string. The reondered result _might_ be easier to read than pre-2.45 version of documentation, but I somehow find the updated SYNOPSIS section almost impossible to work with in the source form. And the need for these otherwise no-op {empty} makes it even less pleasant to work with. I wonder if there is a magic incantation that says "everything should be typeset in monospace in this block, unless ..." so that we can lose all these `back quotes`? And then the part that follows "unless ..." would say how we mark up the <placeholder> part which is the only thing exempt from "everything is in monospace" default. Thanks for a quick response. > Documentation/git-clone.txt | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt > index 5de18de2ab8..8e925db7e9c 100644 > --- a/Documentation/git-clone.txt > +++ b/Documentation/git-clone.txt > @@ -13,9 +13,9 @@ SYNOPSIS > [`-l`] [`-s`] [`--no-hardlinks`] [`-q`] [`-n`] [`--bare`] [`--mirror`] > [`-o` _<name>_] [`-b` _<name>_] [`-u` _<upload-pack>_] [`--reference` _<repository>_] > [`--dissociate`] [`--separate-git-dir` _<git-dir>_] > - [`--depth` _<depth>_] [`--`[`no-`]`single-branch`] [`--no-tags`] > - [++--recurse-submodules++[++=++__<pathspec>__]] [`--`[`no-`]`shallow-submodules`] > - [`--`[`no-`]`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]`reject-shallow`] > + [`--depth` _<depth>_] [`--`[`no-`]{empty}`single-branch`] [`--no-tags`] > + [++--recurse-submodules++[++=++__<pathspec>__]] [++--++[++no-++]{empty}++shallow-submodules++] > + [`--`[`no-`]{empty}`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]{empty}`reject-shallow`] > [++--filter=++__<filter-spec>__] [`--also-filter-submodules`]] [`--`] _<repository>_ > [_<directory>_] > > > base-commit: a7dae3bdc8b516d36f630b12bb01e853a667e0d9
Junio C Hamano <gitster@pobox.com> writes: > The reondered result _might_ be easier to read than pre-2.45 version > of documentation, but I somehow find the updated SYNOPSIS section > almost impossible to work with in the source form. And the need for > these otherwise no-op {empty} makes it even less pleasant to work > with. > > I wonder if there is a magic incantation that says "everything > should be typeset in monospace in this block, unless ..." so that we > can lose all these `back quotes`? And then the part that follows > "unless ..." would say how we mark up the <placeholder> part which > is the only thing exempt from "everything is in monospace" default. > > Thanks for a quick response. > >> Documentation/git-clone.txt | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) Ah, another and *more* important thing. The original series that added the new mark-up to "clone" and "init" updated them as examples to illustrate the rules added by c42ea604 (doc: rework CodingGuidelines with new formatting rules, 2024-03-29) to Documentation/CodingGuidelines. I _dislike_ the idea that we have to sprinkle otherwise no-op {empty} all over the place, if we were to update the SYNOPSIS part of all other commands consistently, but if that is what it takes, we _should_ document that they need to do so (and no, we should not assume that those who document the commands _know_ why asciidoctor wants to eat the [string] there and we should not expect them to insert {empty} without being told). Thanks. >> >> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt >> index 5de18de2ab8..8e925db7e9c 100644 >> --- a/Documentation/git-clone.txt >> +++ b/Documentation/git-clone.txt >> @@ -13,9 +13,9 @@ SYNOPSIS >> [`-l`] [`-s`] [`--no-hardlinks`] [`-q`] [`-n`] [`--bare`] [`--mirror`] >> [`-o` _<name>_] [`-b` _<name>_] [`-u` _<upload-pack>_] [`--reference` _<repository>_] >> [`--dissociate`] [`--separate-git-dir` _<git-dir>_] >> - [`--depth` _<depth>_] [`--`[`no-`]`single-branch`] [`--no-tags`] >> - [++--recurse-submodules++[++=++__<pathspec>__]] [`--`[`no-`]`shallow-submodules`] >> - [`--`[`no-`]`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]`reject-shallow`] >> + [`--depth` _<depth>_] [`--`[`no-`]{empty}`single-branch`] [`--no-tags`] >> + [++--recurse-submodules++[++=++__<pathspec>__]] [++--++[++no-++]{empty}++shallow-submodules++] >> + [`--`[`no-`]{empty}`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]{empty}`reject-shallow`] >> [++--filter=++__<filter-spec>__] [`--also-filter-submodules`]] [`--`] _<repository>_ >> [_<directory>_] >> >> >> base-commit: a7dae3bdc8b516d36f630b12bb01e853a667e0d9
Le dimanche 21 juillet 2024, 01:16:02 CEST Junio C Hamano a écrit : > "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr> > > > > Asciidoc.py does not have the concept of generalized roles, whereas > > asciidoctor interprets [foo]`blah` as blah with role foo in the > > synopsis, making in effect foo disappear in the output. Note that > > square brackets not directly followed by an inline markup do not > > define a role, which is why we do not have the issue on other parts of > > the documentation. > > This was utterly misleading, at least to me, and it took a few times > of reading for me to really understand what you wanted to say. > > It does not matter that "generalized roles" is not known by > AsciiDoc. What really causes the breakage is that a string inside > [square brackets] followed by string inside `a pair of back quotes` > is interpreted by Asciidoctor as "generalized roles" (whatever it > is), even though we do not care at all about such a feature here. > Sorry for not being clear. Indeed I was wrong, Asciidoc.py also has this role management behavior for any other inline markup (++, _, *, ^, ~) except for back-quoted text. > How about phrasing it more like so? > > Writing a string inside [square brackets], immediately followed > by a string inside `a pair of back quotes`, causes asciidoctor > to eliminate the string inside [square brackets], because it is > a syntax to trigger a "generalized role" feature, which we do > not care about in the context of the synopsis section here. > > Work it around by inserting an otherwise no-op {empty} string to > forbid asciidoctor from triggering that feature here. AsciiDoc > is not affected negatively by this additional empty string. > OK, but let's get rid of the "generalized role" stuff, then. > The reondered result _might_ be easier to read than pre-2.45 version > of documentation, but I somehow find the updated SYNOPSIS section > almost impossible to work with in the source form. And the need for > these otherwise no-op {empty} makes it even less pleasant to work > with. I can only agree. The fact that this "feature" is too easily triggered almost voids all the "painless technical doc editing for the rest of us" argument that is the main selling point of Asciidoc, Markdown and other lightweight markup systems. > > I wonder if there is a magic incantation that says "everything > should be typeset in monospace in this block, unless ..." so that we > can lose all these `back quotes`? And then the part that follows > "unless ..." would say how we mark up the <placeholder> part which > is the only thing exempt from "everything is in monospace" default. While doing the styling of synopsis, I tried to be smarter than that. There are basically 3 semantic entities in the grammar: * the _<placeholders>_ in italic * the `keywords`, in monospace * the grammar signs: [, ], |, ..., (, ), etc. These signs are not typeset. Setting everything in monospace would mix keywords and grammar. With this schema in mind, I don't find difficult to understand how the synopsis is written (putting aside the {empty} hack). Fair enough, this is more difficult than just plain text, but the aim is still to get decent output.
Jean-Noël AVILA <jn.avila@free.fr> writes: > Sorry for not being clear. Indeed I was wrong, Asciidoc.py also has this role > management behavior for any other inline markup (++, _, *, ^, ~) except for > back-quoted text. > >> How about phrasing it more like so? >> >> Writing a string inside [square brackets], immediately followed >> by a string inside `a pair of back quotes`, causes asciidoctor >> to eliminate the string inside [square brackets], because it is >> a syntax to trigger a "generalized role" feature, which we do >> not care about in the context of the synopsis section here. >> >> Work it around by inserting an otherwise no-op {empty} string to >> forbid asciidoctor from triggering that feature here. AsciiDoc >> is not affected negatively by this additional empty string. >> > > OK, but let's get rid of the "generalized role" stuff, then. I agree it is not relevant what the feature is called, as that is something we did not want to trigger and take advantage of. It still is necessary to mention the fact that [strings] are eaten by us unknowingly triggering the feature. > While doing the styling of synopsis, I tried to be smarter than that. There > are basically 3 semantic entities in the grammar: > > * the _<placeholders>_ in italic > * the `keywords`, in monospace > * the grammar signs: [, ], |, ..., (, ), etc. These signs are not typeset. > > Setting everything in monospace would mix keywords and grammar. > > With this schema in mind, I don't find difficult to understand how the synopsis > is written (putting aside the {empty} hack). Fair enough, this is more > difficult than just plain text, but the aim is still to get decent output. Thanks. It appears that asciidoctor considers `monospaced` that results in <code>...</code> is a bad match in the SYNOPSIS section cf. https://lore.kernel.org/git/xmqqsew3hdmv.fsf@gitster.g/ but we should be able to sort it out.
Le 22/07/2024 à 18:39, Junio C Hamano a écrit : > Jean-Noël AVILA <jn.avila@free.fr> writes: > >> Sorry for not being clear. Indeed I was wrong, Asciidoc.py also has this role >> management behavior for any other inline markup (++, _, *, ^, ~) except for >> back-quoted text. >> >>> How about phrasing it more like so? >>> >>> Writing a string inside [square brackets], immediately followed >>> by a string inside `a pair of back quotes`, causes asciidoctor >>> to eliminate the string inside [square brackets], because it is >>> a syntax to trigger a "generalized role" feature, which we do >>> not care about in the context of the synopsis section here. >>> >>> Work it around by inserting an otherwise no-op {empty} string to >>> forbid asciidoctor from triggering that feature here. AsciiDoc >>> is not affected negatively by this additional empty string. >>> >> >> OK, but let's get rid of the "generalized role" stuff, then. > > I agree it is not relevant what the feature is called, as that is > something we did not want to trigger and take advantage of. > > It still is necessary to mention the fact that [strings] are eaten > by us unknowingly triggering the feature. > >> While doing the styling of synopsis, I tried to be smarter than that. There >> are basically 3 semantic entities in the grammar: >> >> * the _<placeholders>_ in italic >> * the `keywords`, in monospace >> * the grammar signs: [, ], |, ..., (, ), etc. These signs are not typeset. >> >> Setting everything in monospace would mix keywords and grammar. >> >> With this schema in mind, I don't find difficult to understand how the synopsis >> is written (putting aside the {empty} hack). Fair enough, this is more >> difficult than just plain text, but the aim is still to get decent output. > > Thanks. > > It appears that asciidoctor considers `monospaced` that results in > <code>...</code> is a bad match in the SYNOPSIS section > > cf. https://lore.kernel.org/git/xmqqsew3hdmv.fsf@gitster.g/ > > but we should be able to sort it out. > Please hold on this patch. Cranking on your reflections about the ugly markup and upon advice from Dan Allen (of asciidoctor) [1], I'd like to push another way of managing the files, which would be to define a custom 'synopsis' paragraph style which would allow to process automatically the grammar. Thanks [1] https://asciidoctor.zulipchat.com/#narrow/stream/279642-users/topic/Is.20there.20a.20way.20to.20disable.20role.20attributes.3F
Jean-Noël Avila <jn.avila@free.fr> writes: > Please hold on this patch. Cranking on your reflections about the ugly > markup and upon advice from Dan Allen (of asciidoctor) [1], I'd like to > push another way of managing the files, which would be to define a > custom 'synopsis' paragraph style which would allow to process > automatically the grammar. Sounds good. Anything that lets documenters write what they mean without involving an absolute nightmare of a markup sequence is what we want here. But I'd like to see such a larger scale work early in the next cycle, not close to the end of this cycle a few days before -rc2. In the meantime, I am tempted to (1) apply Dscho's CSS change (but with fixes to avoid "make distclean" issue) and leaving git-clone as-is. or (2) revert the git-clone and git-init mark-up patches (76880f05 (doc: git-clone: apply new documentation formatting guidelines, 2024-03-29) and (5cf7dfe9 (doc: git-init: apply new documentation formatting guidelines, 2024-03-29), which suffers from the <code> being blocks. This will allow us not to worry about CSS change proposed by Dscho this close to the final. or (3) do nothing. I'd probably do (1). Even though chances of unintended regression might be smaller with (2), which would only affect clone and init manual pages (as opposed to anything that uses <code> inside <pre>), it's less work (and more work to validate the result visually, which may be a pain). > [1] > https://asciidoctor.zulipchat.com/#narrow/stream/279642-users/topic/Is.20there.20a.20way.20to.20disable.20role.20attributes.3F I've always felt [verse] was a bad choice (I honestly do not know where its use came from), and I am glad to finally find somebody agreeing ;-) Thanks.
Junio C Hamano <gitster@pobox.com> writes: > In the meantime, I am tempted to > > (1) apply Dscho's CSS change (but with fixes to avoid "make > distclean" issue) and leaving git-clone as-is. or > ... > I'd probably do (1). Even though chances of unintended regression > might be smaller with (2), which would only affect clone and init > manual pages (as opposed to anything that uses <code> inside <pre>), > it's less work (and more work to validate the result visually, which > may be a pain). So, I decided to keep Dscho's latest CSS-workaround change intact, and queued a stupid and obvious workaround clearly marked as a tentative measure on top of it. I am planning to merge these two patches down for the release candidates. Without your incremental update to git-clone.txt, git-clone.html rendered through asciidoctor will say wrong things, so we'd need to apply it anyway. So I cannot put your patch _on_ _hold_ for the purpose of the upcoming release, if we wanted to ship a set of source that produces correct version of git-clone.html. So in the end, we'd need both ;-). Thanks, both. ----- >8 ---------- >8 ---------- >8 ---------- >8 ----- Subject: [PATCH] Doc: fix Asciidoctor css workaround The previous step introduced docinfo.html to be used to tweak the CSS used by the asciidoctor, that by default renders <code> inside <pre> as a block element, breaking the SYNOPSIS section of a few pages that adopted a new convention we use since Git 2.45. But in this project, HTML files are all generated. We do not force any human to write HTML by hand, which is an unusual and cruel punishment. "*.html" is in the .gitignore file, and "make clean" removes them. Having a tracked .html file makes "make clean" make the tree dirty by removing the tracked docinfo.html file. Let's do an obvious, minimum and stupid workaround to generate that file at runtime instead. The mark-up is being rethought in a major way for the next development cycle, so what the CSS workaround we added in the previous step may have to adjusted, possibly in a large way, anyway. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/.gitignore | 1 - Documentation/Makefile | 5 +++++ Documentation/{docinfo.html => docinfo-html.in} | 0 3 files changed, 5 insertions(+), 1 deletion(-) rename Documentation/{docinfo.html => docinfo-html.in} (100%) diff --git a/Documentation/.gitignore b/Documentation/.gitignore index d11567fbbe..a48448de32 100644 --- a/Documentation/.gitignore +++ b/Documentation/.gitignore @@ -1,6 +1,5 @@ *.xml *.html -!/docinfo.html *.[1-8] *.made *.texi diff --git a/Documentation/Makefile b/Documentation/Makefile index 78e407e4bd..371d56eb5e 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -209,6 +209,8 @@ XMLTO_EXTRA += --skip-validation XMLTO_EXTRA += -x manpage.xsl endif +ASCIIDOC_DEPS += docinfo.html + SHELL_PATH ?= $(SHELL) # Shell quote; SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) @@ -337,6 +339,9 @@ clean: $(RM) $(cmds_txt) $(mergetools_txt) *.made $(RM) GIT-ASCIIDOCFLAGS +docinfo.html: docinfo-html.in + $(QUIET_GEN)$(RM) $@ && cat $< >$@ + $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS) $(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@ $< diff --git a/Documentation/docinfo.html b/Documentation/docinfo-html.in similarity index 100% rename from Documentation/docinfo.html rename to Documentation/docinfo-html.in
On Tue, Jul 23, 2024 at 1:44 PM Junio C Hamano <gitster@pobox.com> wrote: > Subject: [PATCH] Doc: fix Asciidoctor css workaround > > The previous step introduced docinfo.html to be used to tweak the > CSS used by the asciidoctor, that by default renders <code> inside > <pre> as a block element, breaking the SYNOPSIS section of a few > pages that adopted a new convention we use since Git 2.45. > > But in this project, HTML files are all generated. We do not force > any human to write HTML by hand, which is an unusual and cruel > punishment. "*.html" is in the .gitignore file, and "make clean" > removes them. Having a tracked .html file makes "make clean" make > the tree dirty by removing the tracked docinfo.html file. > > Let's do an obvious, minimum and stupid workaround to generate that > file at runtime instead. The mark-up is being rethought in a major > way for the next development cycle, so what the CSS workaround we Perhaps? s/what/that/ > added in the previous step may have to adjusted, possibly in a large > way, anyway. > > Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine <sunshine@sunshineco.com> writes: > On Tue, Jul 23, 2024 at 1:44 PM Junio C Hamano <gitster@pobox.com> wrote: >> Subject: [PATCH] Doc: fix Asciidoctor css workaround >> >> The previous step introduced docinfo.html to be used to tweak the >> CSS used by the asciidoctor, that by default renders <code> inside >> <pre> as a block element, breaking the SYNOPSIS section of a few >> pages that adopted a new convention we use since Git 2.45. >> >> But in this project, HTML files are all generated. We do not force >> any human to write HTML by hand, which is an unusual and cruel >> punishment. "*.html" is in the .gitignore file, and "make clean" >> removes them. Having a tracked .html file makes "make clean" make >> the tree dirty by removing the tracked docinfo.html file. >> >> Let's do an obvious, minimum and stupid workaround to generate that >> file at runtime instead. The mark-up is being rethought in a major >> way for the next development cycle, so what the CSS workaround we > > Perhaps? s/what/that/ Thanks, I ended up doing "so what" -> "and". >> added in the previous step may have to adjusted, possibly in a large >> way, anyway. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 5de18de2ab8..8e925db7e9c 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -13,9 +13,9 @@ SYNOPSIS [`-l`] [`-s`] [`--no-hardlinks`] [`-q`] [`-n`] [`--bare`] [`--mirror`] [`-o` _<name>_] [`-b` _<name>_] [`-u` _<upload-pack>_] [`--reference` _<repository>_] [`--dissociate`] [`--separate-git-dir` _<git-dir>_] - [`--depth` _<depth>_] [`--`[`no-`]`single-branch`] [`--no-tags`] - [++--recurse-submodules++[++=++__<pathspec>__]] [`--`[`no-`]`shallow-submodules`] - [`--`[`no-`]`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]`reject-shallow`] + [`--depth` _<depth>_] [`--`[`no-`]{empty}`single-branch`] [`--no-tags`] + [++--recurse-submodules++[++=++__<pathspec>__]] [++--++[++no-++]{empty}++shallow-submodules++] + [`--`[`no-`]{empty}`remote-submodules`] [`--jobs` _<n>_] [`--sparse`] [`--`[`no-`]{empty}`reject-shallow`] [++--filter=++__<filter-spec>__] [`--also-filter-submodules`]] [`--`] _<repository>_ [_<directory>_]