| Message ID | 20230323221523.52472-1-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8806120de6c242a7143cfb1701106c5f9f77cf90 |
| Headers | show |
| Series | doc: asciidoc: remove custom header macro | expand |
On Thu, Mar 23, 2023 at 04:15:23PM -0600, Felipe Contreras wrote: > In 2007 we added a custom header macro to provide version information > 7ef195ba3e (Documentation: Add version information to man pages, > 2007-03-25), > > However, in 2008 asciidoc added the attributes to do this properly [1]. > > This was not implemented in Git until 2019: 226daba280 (Doc/Makefile: > give mansource/-version/-manual attributes, 2019-09-16). > > But in 2023 we are doing it properly, so there's no need for the custom > macro. > > [1] https://github.com/asciidoc-py/asciidoc-py/commit/ad78a3c This should be OK to do, as it is just touching the python asciidoc side. When we discussed those attributes in 2019: https://lore.kernel.org/git/20190320183229.GK31362@pobox.com/ asciidoctor support was new and incomplete. It needed v1.5.7 (from 2018), and even today still does not seem to handle manversion. But since this patch leaves in place the equivalent hack in asciidoctor-extensions.rb, it will continue working. Aside: If we think asciidoctor 1.5.7 is recent enough to rely on, then we might want to simplify our hack to just output manversion. Right now we generate duplicate "source" and "manual" lines, though docbook seems to do OK with it. -Peff
Jeff King <peff@peff.net> writes: > On Thu, Mar 23, 2023 at 04:15:23PM -0600, Felipe Contreras wrote: > >> In 2007 we added a custom header macro to provide version information >> 7ef195ba3e (Documentation: Add version information to man pages, >> 2007-03-25), >> >> However, in 2008 asciidoc added the attributes to do this properly [1]. >> >> This was not implemented in Git until 2019: 226daba280 (Doc/Makefile: >> give mansource/-version/-manual attributes, 2019-09-16). >> >> But in 2023 we are doing it properly, so there's no need for the custom >> macro. >> >> [1] https://github.com/asciidoc-py/asciidoc-py/commit/ad78a3c > > This should be OK to do, as it is just touching the python asciidoc > side. When we discussed those attributes in 2019: > > https://lore.kernel.org/git/20190320183229.GK31362@pobox.com/ > > asciidoctor support was new and incomplete. It needed v1.5.7 (from > 2018), and even today still does not seem to handle manversion. But > since this patch leaves in place the equivalent hack in > asciidoctor-extensions.rb, it will continue working. Sounds like the proposed log message can use a bit more polishing to help future readers of "git log", then. But I think it is not required to be explicit about us leaving the asciidoctor side untouched to keep it working (in other words, we do not talk about things that we are not doing in our log message, unless it is so unnatural not to do them at the same time to warrant such an explanation). Will queue with your Reviewed-by: in the morning. Thanks, both.
On Wed, Apr 5, 2023 at 10:57 PM Jeff King <peff@peff.net> wrote: > > On Thu, Mar 23, 2023 at 04:15:23PM -0600, Felipe Contreras wrote: > > > In 2007 we added a custom header macro to provide version information > > 7ef195ba3e (Documentation: Add version information to man pages, > > 2007-03-25), > > > > However, in 2008 asciidoc added the attributes to do this properly [1]. > > > > This was not implemented in Git until 2019: 226daba280 (Doc/Makefile: > > give mansource/-version/-manual attributes, 2019-09-16). > > > > But in 2023 we are doing it properly, so there's no need for the custom > > macro. > > > > [1] https://github.com/asciidoc-py/asciidoc-py/commit/ad78a3c > > This should be OK to do, as it is just touching the python asciidoc > side. When we discussed those attributes in 2019: > > https://lore.kernel.org/git/20190320183229.GK31362@pobox.com/ > > asciidoctor support was new and incomplete. It needed v1.5.7 (from > 2018), Except that isn't true. The `manmanual` and `mansource` attributes were supported since day 1 [1], back in 2015 and included in v1.5.3. But for the manpage backend, which we refuse to use. It's true they were added in 2018 and in v1.5.7 for the docbook5 backend [2], which is what we use when we do USE_ASCIIDOCTOR=y, but we could have used the manpage backend instead, which already had support for that 3 years prior. > and even today still does not seem to handle manversion. Why do we need `manversion`? All that it's used for is in the DocBook Stylesheets to join the source name and the version, even its own documentation explains what it looks like in practice [3]: In practice, there are many pages that simply have a version number in the "source" field. So, it looks like what we have is a two-part field, Name Version So if we have `source="Git"`, and `version="2.4.0"`, we can just have `source="Git 2.40.0"`. Why do we have to split that information only for the DocBook Stylesheets to join it in? > Aside: If we think asciidoctor 1.5.7 is recent enough to rely on, then > we might want to simplify our hack to just output manversion. There is no need for any hack: we can just set the "mansource" attribute to "Git $(GIT_VERSION)", and everything will work correctly for both asciidoc and asciidoctor in all backends. Why do we insist on hacks for asciidoc.py/2007 and asciidoc|docbook5/2017? Especially when I sent the fix for *everything* in 2021 [4]. This is absolutely no reason to complicate the joining of two strings this much. Cheers. [1] https://github.com/asciidoctor/asciidoctor/commit/7bddc416 [2] https://github.com/asciidoctor/asciidoctor/commit/6b07b0ba [3] https://docbook.sourceforge.net/release/xsl/current/doc/common/template.get.refentry.source.html [4] https://lore.kernel.org/git/20210514121435.504423-7-felipe.contreras@gmail.com/
On Thu, Apr 06, 2023 at 01:18:36AM -0500, Felipe Contreras wrote: > > and even today still does not seem to handle manversion. > > Why do we need `manversion`? > > All that it's used for is in the DocBook Stylesheets to join the > source name and the version, even its own documentation explains what > it looks like in practice [3]: > > In practice, there are many pages that simply have a version number > in the "source" field. So, it looks like what we have is a two-part > field, Name Version > > So if we have `source="Git"`, and `version="2.4.0"`, we can just have > `source="Git 2.40.0"`. > > Why do we have to split that information only for the DocBook > Stylesheets to join it in? I don't know of any particular reason why we couldn't put both in the source field. I had forgotten we discussed this 2 years ago. > > Aside: If we think asciidoctor 1.5.7 is recent enough to rely on, then > > we might want to simplify our hack to just output manversion. > > There is no need for any hack: we can just set the "mansource" > attribute to "Git $(GIT_VERSION)", and everything will work correctly > for both asciidoc and asciidoctor in all backends. > > Why do we insist on hacks for asciidoc.py/2007 and asciidoc|docbook5/2017? > > Especially when I sent the fix for *everything* in 2021 [4]. You say "insist" like somebody is arguing for it. It looks like the series you linked got some review comments, and you followed-up. I didn't carefully read the re-rolls (then or now), but the original patches seem like a good direction to me. Looking at the timing in the archive, I suspect that inter-personal drama in other threads caused people not to read those re-rolls. At any rate, I don't think any of that needs to hold up this patch, which is not touching the asciidoctor side at all (I only wondered while reviewing it what the implications might be). -Peff
On Wed, Apr 05, 2023 at 09:22:03PM -0700, Junio C Hamano wrote: > Sounds like the proposed log message can use a bit more polishing to > help future readers of "git log", then. But I think it is not > required to be explicit about us leaving the asciidoctor side > untouched to keep it working (in other words, we do not talk about > things that we are not doing in our log message, unless it is so > unnatural not to do them at the same time to warrant such an > explanation). Yes, I think the "is it unnatural" is a debatable point here. I'm OK with the commit message as-is. > Will queue with your Reviewed-by: in the morning. Sounds good, thanks. -Peff
On Wed, Apr 5, 2023 at 11:22 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jeff King <peff@peff.net> writes: > > > On Thu, Mar 23, 2023 at 04:15:23PM -0600, Felipe Contreras wrote: > > > >> In 2007 we added a custom header macro to provide version information > >> 7ef195ba3e (Documentation: Add version information to man pages, > >> 2007-03-25), > >> > >> However, in 2008 asciidoc added the attributes to do this properly [1]. > >> > >> This was not implemented in Git until 2019: 226daba280 (Doc/Makefile: > >> give mansource/-version/-manual attributes, 2019-09-16). > >> > >> But in 2023 we are doing it properly, so there's no need for the custom > >> macro. > >> > >> [1] https://github.com/asciidoc-py/asciidoc-py/commit/ad78a3c > > > > This should be OK to do, as it is just touching the python asciidoc > > side. When we discussed those attributes in 2019: > > > > https://lore.kernel.org/git/20190320183229.GK31362@pobox.com/ > > > > asciidoctor support was new and incomplete. It needed v1.5.7 (from > > 2018), and even today still does not seem to handle manversion. But > > since this patch leaves in place the equivalent hack in > > asciidoctor-extensions.rb, it will continue working. > > Sounds like the proposed log message can use a bit more polishing to > help future readers of "git log", then. Yes, *if* what Jeff King was saying were true, which is not. At least not completely. > But I think it is not required to be explicit about us leaving the > asciidoctor side untouched to keep it working (in other words, we do > not talk about things that we are not doing in our log message, unless > it is so unnatural not to do them at the same time to warrant such an > explanation). I did explain that we could fix all the issues of this particular problem for both asciidoc and asciidoctor as far back as 2021 [1]. This fix did not materialize in subsequent patches: [2][3][4][5][6]. But the fix exists, and it's explicitly explained (multiple times). Basically: instead of trying to make docbook-xsl understand "$version" in order to join it as "$source $version". We can just say `source="$source $version"` and forget about $version. I don't know how I can say it more clearly: we can fix all the problems for all the tools and all their backends with one simple patch. So after 6 attempts I'm trying to change the strategy and only clarify the problem for asciidoc.py, which hopefully is clear now. But it can be fixed for asciidoctor as well...*today*. Cheers. [1] https://lore.kernel.org/git/20210514121435.504423-7-felipe.contreras@gmail.com/ [2] https://lore.kernel.org/git/20210515115653.922902-4-felipe.contreras@gmail.com [3] https://lore.kernel.org/git/20210521223701.526547-4-felip01e.contreras@gmail.com/ [4] https://lore.kernel.org/git/20210608090026.1737348-4-felipe.contreras@gmail.com/ [5] https://lore.kernel.org/git/20210618215231.796592-420-felipe.contreras@gmail.com/ [6] https://lore.kernel.org/git/20210621163110.1074145-4-felipe.contreras@gmail.com/
Jeff King <peff@peff.net> writes: > At any rate, I don't think any of that needs to hold up this patch, > which is not touching the asciidoctor side at all (I only wondered while > reviewing it what the implications might be). Yeah, sounds good. Wondering about and discussing possible implications is also a good thing, when done in a productive way. Queued. Trickling in modernization a bit by bit to avoid hogging too much list bandwidth may also be a good thing to do. Thanks.
diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf index 3e4c13971b..60f76f43ed 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -51,25 +51,6 @@ ifdef::doctype-manpage[] endif::doctype-manpage[] endif::backend-docbook[] -ifdef::doctype-manpage[] -ifdef::backend-docbook[] -[header] -template::[header-declarations] -<refentry> -<refmeta> -<refentrytitle>{mantitle}</refentrytitle> -<manvolnum>{manvolnum}</manvolnum> -<refmiscinfo class="source">{mansource}</refmiscinfo> -<refmiscinfo class="version">{manversion}</refmiscinfo> -<refmiscinfo class="manual">{manmanual}</refmiscinfo> -</refmeta> -<refnamediv> - <refname>{manname}</refname> - <refpurpose>{manpurpose}</refpurpose> -</refnamediv> -endif::backend-docbook[] -endif::doctype-manpage[] - ifdef::backend-xhtml11[] [attributes] git-relative-html-prefix=
In 2007 we added a custom header macro to provide version information 7ef195ba3e (Documentation: Add version information to man pages, 2007-03-25), However, in 2008 asciidoc added the attributes to do this properly [1]. This was not implemented in Git until 2019: 226daba280 (Doc/Makefile: give mansource/-version/-manual attributes, 2019-09-16). But in 2023 we are doing it properly, so there's no need for the custom macro. [1] https://github.com/asciidoc-py/asciidoc-py/commit/ad78a3c Cc: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- Documentation/asciidoc.conf | 19 ------------------- 1 file changed, 19 deletions(-)