{"thread":{"id":"55690","subject":"[PATCH 00/11] doc: asciidoctor: direct man page creation and fixes","startedAt":"2021-05-14T12:14:44Z","lastAt":"2021-05-17T16:51:49Z","messageCount":25,"participants":["Felipe Contreras","Martin Ågren","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"424564","messageId":"20210514121435.504423-1-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":null,"subject":"[PATCH 00/11] doc: asciidoctor: direct man page creation and fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:24Z","receivedAt":"2021-05-14T12:14:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This patch series enables direct man page creation with asciidoctor, but in addition tries to\nminimize the difference with asciidoc+docbook.\n\nI fixed as many issues as I could, and now the doc-diff looks very sensible. I could not find any\nmajor issues, however, some minor issues still remain.\n\nOn the other hand the asciidoc method has issues as well, so it's not clear which method is superior\nat this point; both have advantages and disadvantages.\n\nAt the very least the man pages with pure asciidoctor should be quite usable now.\n\nThis series builds on top of my cleanups [1]. It's probably not worth the effort to split the two.\n\n[1] https://lore.kernel.org/git/20210514115631.503276-1-felipe.contreras@gmail.com\n\nFelipe Contreras (11):\n doc: allow the user to provide ASCIIDOC_EXTRA\n doc: doc-diff: allow more than one flag\n doc: doc-diff: set docdate manually\n doc: use asciidoctor to build man pages directly\n doc: asciidoctor: add linkgit macros in man pages\n doc: join mansource and manversion\n doc: add man pages workaround for asciidoctor\n doc: asciidoctor: add hack for xrefs\n doc: asciidoctor: add hack to improve links\n doc: asciidoctor: add support for baseurl\n doc: asciidoctor: cleanup man page hack\n\n Documentation/Makefile | 24 ++++++++-----\n Documentation/asciidoctor-extensions.rb | 46 +++++++++++++++++++++++++\n Documentation/doc-diff | 8 ++---\n 3 files changed, 65 insertions(+), 13 deletions(-)\n\n-- \n2.31.1\n\n"},{"id":"424565","messageId":"20210514121435.504423-2-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:25Z","receivedAt":"2021-05-14T12:14:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Without `override` all additions will be ignored by make.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/Makefile | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex f5605b7767..981e322f18 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -191,9 +191,9 @@ ASCIIDOC = asciidoctor\n ASCIIDOC_CONF =\n ASCIIDOC_HTML = xhtml5\n ASCIIDOC_DOCBOOK = docbook5\n-ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n-ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n-ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n+override ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n+override ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n+override ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n ASCIIDOC_DEPS = asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n@@ -206,12 +206,12 @@ SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n \n ifdef DEFAULT_PAGER\n DEFAULT_PAGER_SQ = $(subst ','\\'',$(DEFAULT_PAGER))\n-ASCIIDOC_EXTRA += -a 'git-default-pager=$(DEFAULT_PAGER_SQ)'\n+override ASCIIDOC_EXTRA += -a 'git-default-pager=$(DEFAULT_PAGER_SQ)'\n endif\n \n ifdef DEFAULT_EDITOR\n DEFAULT_EDITOR_SQ = $(subst ','\\'',$(DEFAULT_EDITOR))\n-ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n+override ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n endif\n \n QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n@@ -375,7 +375,7 @@ technical/api-index.txt: technical/api-index-skel.txt \\\n \ttechnical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))\n \t$(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh\n \n-technical/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../\n+technical/%.html: override ASCIIDOC_EXTRA += -a git-relative-html-prefix=../\n $(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt \\\n \tasciidoc.conf GIT-ASCIIDOCFLAGS\n \t$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt\n@@ -425,7 +425,7 @@ $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt\n \n WEBDOC_DEST = /pub/software/scm/git/docs\n \n-howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../\n+howto/%.html: override ASCIIDOC_EXTRA += -a git-relative-html-prefix=../\n $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS\n \t$(QUIET_ASCIIDOC) \\\n \tsed -e '1,/^$$/d' $< | \\\n-- \n2.31.1\n\n"},{"id":"424566","messageId":"20210514121435.504423-3-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 02/11] doc: doc-diff: allow more than one flag","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:26Z","receivedAt":"2021-05-14T12:14:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n Documentation/doc-diff | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex 1694300e50..ecd88b0524 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -146,7 +146,7 @@ render_tree () {\n \t# through.\n \toid=$1 &&\n \tdname=$2 &&\n-\tmakemanflags=$3 &&\n+\tmakemanflags=\"$3\" &&\n \tif ! test -d \"$tmp/installed/$dname\"\n \tthen\n \t\tgit -C \"$tmp/worktree\" checkout --detach \"$oid\" &&\n@@ -181,6 +181,6 @@ render_tree () {\n \tfi\n }\n \n-render_tree $from_oid $from_dir $from_makemanflags &&\n-render_tree $to_oid $to_dir $to_makemanflags &&\n+render_tree $from_oid $from_dir \"$from_makemanflags\" &&\n+render_tree $to_oid $to_dir \"$to_makemanflags\" &&\n git -C $tmp/rendered diff --no-index \"$@\" $from_dir $to_dir\n-- \n2.31.1\n\n"},{"id":"424567","messageId":"20210514121435.504423-4-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 03/11] doc: doc-diff: set docdate manually","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:27Z","receivedAt":"2021-05-14T12:14:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In order to minimize the differences in the footer.\n\nAsciidoc automatically generates a date with format '%Y-%m-%d', while\nasciidoctor '%F'.\n\nI personally prefer the latter, so only modify it for diff purposes.\n\nFixes tons of these:\n\n-Git omitted 01/01/1970 GIT-ADD(1)\n+Git omitted 1970-01-01 GIT-ADD(1)\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/doc-diff | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex ecd88b0524..aae5fc1933 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -111,7 +111,7 @@ construct_makemanflags () {\n \t\techo USE_ASCIIDOCTOR=\n \telif test \"$1\" = \"-asciidoctor\"\n \tthen\n-\t\techo USE_ASCIIDOCTOR=YesPlease\n+\t\techo USE_ASCIIDOCTOR=YesPlease ASCIIDOC_EXTRA='-adocdate=\"01/01/1970\"'\n \tfi\n }\n \n-- \n2.31.1\n\n"},{"id":"424568","messageId":"20210514121435.504423-5-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 04/11] doc: use asciidoctor to build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:28Z","receivedAt":"2021-05-14T12:14:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to use xmlto to build the man pages when modern\nasciidoctor can do it by itself.\n\nThis new mode will be active only when USE_ASCIIDOCTOR is set.\n\nSuggested-by: Bagas Sanjaya \nSigned-off-by: Felipe Contreras \n---\n Documentation/Makefile | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 981e322f18..ce9cea0817 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -198,6 +198,7 @@ ASCIIDOC_DEPS = asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n endif\n \n SHELL_PATH ?= $(SHELL)\n@@ -362,8 +363,13 @@ $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)\n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n+ifdef TXT_TO_MAN\n+%.1 %.5 %.7 : %.txt $(ASCIIDOC_DEPS)\n+\t$(QUIET_ASCIIDOC)$(TXT_TO_MAN) -o $@ $<\n+else\n %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n \t$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n+endif\n \n %.xml : %.txt $(ASCIIDOC_DEPS)\n \t$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<\n-- \n2.31.1\n\n"},{"id":"424569","messageId":"20210514121435.504423-6-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 05/11] doc: asciidoctor: add linkgit macros in man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:29Z","receivedAt":"2021-05-14T12:15:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Fixes the doc-diff:\n\n- Please see git-commit(1) for alternative ways to add content to a\n- commit.\n+ Please see for alternative ways to add content to a commit.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/asciidoctor-extensions.rb | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex d906a00803..ad68f7b0bb 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -15,6 +15,8 @@ module Git\n \"#{target}(#{attrs[1]})\"\n elsif parent.document.basebackend? 'html'\n %(#{target}(#{attrs[1]}))\n+ elsif parent.document.basebackend? 'manpage'\n+ \"\\e\\\\fB%s\\e\\\\fR(%s)\" % [target, attrs[1]]\n elsif parent.document.basebackend? 'docbook'\n \"\\n\" \\\n \"#{target}\" \\\n-- \n2.31.1\n\n"},{"id":"424570","messageId":"20210514121435.504423-7-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 06/11] doc: join mansource and manversion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:30Z","receivedAt":"2021-05-14T12:15:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no real value in having two fields when one does the trick.\n\nAlso, this add the version for asciidoctor generated man pages.\n\nFixes the doc-diff:\n\n -Git omitted 01/01/1970 GIT-ADD(1)\n +Git 01/01/1970 GIT-ADD(1)\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/Makefile | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex ce9cea0817..a514a4e72c 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -137,8 +137,7 @@ ASCIIDOC_HTML = xhtml11\n ASCIIDOC_DOCBOOK = docbook\n ASCIIDOC_CONF = -f asciidoc.conf\n ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \\\n-\t\t-amanversion=$(GIT_VERSION) \\\n-\t\t-amanmanual='Git Manual' -amansource='Git'\n+\t\t-amanmanual='Git Manual' -amansource='Git $(GIT_VERSION)'\n ASCIIDOC_DEPS = asciidoc.conf GIT-ASCIIDOCFLAGS\n TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)\n TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)\n-- \n2.31.1\n\n"},{"id":"424571","messageId":"20210514121435.504423-8-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 07/11] doc: add man pages workaround for asciidoctor","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:31Z","receivedAt":"2021-05-14T12:15:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Currently asciidoctor doesn't convert number character references\n(&#xx;) correctly for man pages.\n\nThis hack fixes the issue with minimum changes elsewhere so it's easy to\nremove when fixed.\n\nFixes doc-diffs like:\n\n so line count cannot be shown) and there is no difference between\n indexed copy and the working tree version (if the working tree\n version were also different, binary would have been shown in place\n- of nothing). The other file, git-add--interactive.perl, has 403\n- lines added and 35 lines deleted if you commit what is in the\n- index, but working tree file has further modifications (one\n+ of nothing). The other file, git-add--interactive.perl,\n+ has 403 lines added and 35 lines deleted if you commit what is in\n+ the index, but working tree file has further modifications (one\n addition and one deletion).\n\nhttps://github.com/asciidoctor/asciidoctor/issues/4059\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/asciidoctor-extensions.rb | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex ad68f7b0bb..11937c2c1d 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -45,6 +45,17 @@ module Git\n end\n \n Asciidoctor::Extensions.register do\n+ # Override attributes for man pages.\n+ # https://github.com/asciidoctor/asciidoctor/issues/4059\n+ tree_processor do\n+ process do |document|\n+ if document.backend == 'manpage'\n+ document.attributes.merge!({ 'litdd' => '\\--', 'plus' => '+' })\n+ end\n+ document\n+ end\n+ end\n+\n inline_macro Git::Documentation::LinkGitProcessor, :linkgit\n postprocessor Git::Documentation::DocumentPostProcessor\n end\n-- \n2.31.1\n\n"},{"id":"424572","messageId":"20210514121435.504423-9-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 08/11] doc: asciidoctor: add hack for xrefs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:32Z","receivedAt":"2021-05-14T12:15:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The docbook manpage stylesheets convert cross-references with format the\n'section called “%t”'. I personally prefer the asciidoctor version, but\nfor now add a hack to minimize the diff.\n\nThanks to the extensibility of Ruby we can override corresponding method\nin the man page converter.\n\nThis fixes doc-diffs like:\n\n --worktree-attributes\n Look for attributes in .gitattributes files in the working tree as\n - well (see the section called “ATTRIBUTES”).\n + well (see ATTRIBUTES).\n\nThis can easily be removed later once we are confortable with the\nasciidoctor version.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/asciidoctor-extensions.rb | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex 11937c2c1d..b2bbb318ad 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -1,5 +1,22 @@\n require 'asciidoctor'\n require 'asciidoctor/extensions'\n+require 'asciidoctor/converter/manpage'\n+\n+module Asciidoctor\n+ class Converter::ManPageConverter\n+ alias orig_convert_inline_anchor convert_inline_anchor\n+ def convert_inline_anchor(node)\n+ case node.type\n+ when :xref\n+ return node.text if node.text\n+ refid = node.attributes['refid']\n+ 'the section called “%s”' % refid.gsub('_', ' ')\n+ else\n+ orig_convert_inline_anchor(node)\n+ end\n+ end\n+ end\n+end\n \n module Git\n module Documentation\n-- \n2.31.1\n\n"},{"id":"424573","messageId":"20210514121435.504423-10-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 09/11] doc: asciidoctor: add hack to improve links","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:33Z","receivedAt":"2021-05-14T12:15:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The way asciidoctor handles links is very primitive compared to docbook.\nLinks are simply presented in the format \"#{text} <#{target}>\", which\nmay not be all that bad for the future, but pollutes the doc-diff.\n\nBy adding another modification to convert_inline_anchor() we can present\nlinks in a form very similar to docbook, diminishing the doc-diff.\n\nThis significantly reduces the doc-diff:\n\nFrom:\n\n abysmal performance). These safety and performance issues cannot be\n backward compatibly fixed and as such, its use is not recommended.\n Please use an alternative history filtering tool such as git\n- filter-repo[1]. If you still need to use git filter-branch, please\n- carefully read the section called “SAFETY” (and the section called\n- “PERFORMANCE”) to learn about the land mines of filter-branch, and then\n- vigilantly avoid as many of the hazards listed there as reasonably\n- possible.\n+ filter-repo\" . If you\n+ still need to use git filter-branch, please carefully read the section\n+ called “SAFETY” (and the section called “PERFORMANCE”) to learn about\n+ the land mines of filter-branch, and then vigilantly avoid as many of\n+ the hazards listed there as reasonably possible.\n\n-NOTES\n- 1. git filter-repo\n- https://github.com/newren/git-filter-repo/\n-\n- 2. filter-lamely\n- https://github.com/newren/git-filter-repo/blob/master/contrib/filter-repo-demos/filter-lamely\n-\n\nTo:\n\n NOTES\n- 1. git filter-repo\n+ [1] git filter-repo\n https://github.com/newren/git-filter-repo/\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/asciidoctor-extensions.rb | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex b2bbb318ad..42133ee6c3 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -11,6 +11,19 @@ module Asciidoctor\n return node.text if node.text\n refid = node.attributes['refid']\n 'the section called “%s”' % refid.gsub('_', ' ')\n+ when :link\n+ return node.target if node.text == node.target\n+ doc = node.document\n+\n+ footnote = doc.footnotes.find { |e| e.id == node.target }\n+ if !footnote\n+ footnote_text = \"%s\\n\\e.RS\\n\\e\\\\%%%s\\n\\e.RE\" % [node.text, node.target]\n+ index = doc.counter('footnote-number')\n+ footnote = Document::Footnote.new(index, node.target, footnote_text)\n+ doc.register(:footnotes, footnote)\n+ end\n+\n+ \"\\e\\\\fB%s\\e\\\\fR[%d]\" % [node.text, footnote.index]\n else\n orig_convert_inline_anchor(node)\n end\n-- \n2.31.1\n\n"},{"id":"424574","messageId":"20210514121435.504423-11-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 10/11] doc: asciidoctor: add support for baseurl","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:34Z","receivedAt":"2021-05-14T12:15:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that we can present relative links correctly.\n\nReduces the doc-diff:\n\n NOTES\n - 1. “Understanding history: What is a branch?”\n - file:///$HOME/share/doc/git-doc/user-manual.html#what-is-a-branch\n + [1] “Understanding history: What is a branch?”\n + user-manual.html#what-is-a-branch\n\n NOTES\n - 1. “Understanding history: What is a branch?”\n + [1] “Understanding history: What is a branch?”\n file:///$HOME/share/doc/git-doc/user-manual.html#what-is-a-branch\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/Makefile | 1 +\n Documentation/asciidoctor-extensions.rb | 4 +++-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a514a4e72c..cc5ff54d92 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -193,6 +193,7 @@ ASCIIDOC_DOCBOOK = docbook5\n override ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n override ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n override ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n+override ASCIIDOC_EXTRA += -abaseurl='$(MAN_BASE_URL)'\n ASCIIDOC_DEPS = asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex 42133ee6c3..2ed92c3055 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -17,7 +17,9 @@ module Asciidoctor\n \n footnote = doc.footnotes.find { |e| e.id == node.target }\n if !footnote\n- footnote_text = \"%s\\n\\e.RS\\n\\e\\\\%%%s\\n\\e.RE\" % [node.text, node.target]\n+ target = node.target\n+ target = doc.attributes['baseurl'] + target unless target.include? ':'\n+ footnote_text = \"%s\\n\\e.RS\\n\\e\\\\%%%s\\n\\e.RE\" % [node.text, target]\n index = doc.counter('footnote-number')\n footnote = Document::Footnote.new(index, node.target, footnote_text)\n doc.register(:footnotes, footnote)\n-- \n2.31.1\n\n"},{"id":"424575","messageId":"20210514121435.504423-12-felipe.contreras@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-1-felipe.contreras@gmail.com","subject":"[PATCH 11/11] doc: asciidoctor: cleanup man page hack","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:14:35Z","receivedAt":"2021-05-14T12:15:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's basically nothing we need from the original\norig_convert_inline_anchor(), so let's remove calls to it.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/asciidoctor-extensions.rb | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex 2ed92c3055..f23c5628a5 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -4,7 +4,6 @@ require 'asciidoctor/converter/manpage'\n \n module Asciidoctor\n class Converter::ManPageConverter\n- alias orig_convert_inline_anchor convert_inline_anchor\n def convert_inline_anchor(node)\n case node.type\n when :xref\n@@ -26,8 +25,10 @@ module Asciidoctor\n end\n \n \"\\e\\\\fB%s\\e\\\\fR[%d]\" % [node.text, footnote.index]\n+ when :ref, :bibref\n+ ''\n else\n- orig_convert_inline_anchor(node)\n+ nil\n end\n end\n end\n-- \n2.31.1\n\n"},{"id":"424590","messageId":"CAN0heSpX53tK8Z4XSx4sp79b+XWKZg5+ABW8pmzBSHPZ+qy+oQ@mail.gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-5-felipe.contreras@gmail.com","subject":"Re: [PATCH 04/11] doc: use asciidoctor to build man pages directly","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-05-14T15:38:46Z","receivedAt":"2021-05-14T15:39:01Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 14 May 2021 at 14:14, Felipe Contreras\n wrote:\n>\n> There's no need to use xmlto to build the man pages when modern\n> asciidoctor can do it by itself.\n>\n> This new mode will be active only when USE_ASCIIDOCTOR is set.\n\nMay I suggest incorporating something more like brian's patch here [1],\nso that there's a separate knob for this thing?\n\nThe commit message is short on details and makes it sound like this is\nit, we're done. But then there are several patches to fix up things.\nWhich is a good approach, so that this patch doesn't need to do several\nthings at once. This commit message could say something about it, e.g.,\n\n The doc-diff here [which doc-diff? see below] is a XYZ-line diff.\n Large parts of this difference will be addressed in the following\n patches.\n\nAbout the use of doc-diff: If this commit introduces a new knob rather\nthan taking over USE_ASCIIDOCTOR=Yes, the next patch could be Peff's\npatch to doc-diff that compares the two asciidoctor approaches [2], and\nthen the next few patches could diff between them. That would get the\nasciidoc-vs-asciidoctor differences out of the way, so you can focus on\nasciidoctor-vs-asciidoctor.\n\nWith a separate knob, it would feel like a lot easier decision to take\nsomething like this. There are over 11000 lines in the doc-diff after\napplying your series, and there's the missing boldness for literals.\nMaybe those differences are all great (I would be missing the bold\nliterals, though). If this series doesn't affect someone using\n\"vanilla\" USE_ASCIIDOCTOR=Yes, we could let this thing cook in master\nand work incrementally on top.\n\n[1] https://lore.kernel.org/git/20210514003104.94644-2-sandals@crustytoothpaste.net/\n\n[2] https://lore.kernel.org/git/YJt81neO7zsGz2ah@coredump.intra.peff.net/\n\n\nMartin\n"},{"id":"424591","messageId":"20210514154316.810-1-martin.agren@gmail.com","threadId":"55690","inReplyTo":"20210514121435.504423-4-felipe.contreras@gmail.com","subject":"Re: [PATCH 03/11] doc: doc-diff: set docdate manually","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-05-14T15:43:16Z","receivedAt":"2021-05-14T15:43:41Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 14 May 2021 at 14:14, Felipe Contreras wrote:\n>\n> Asciidoc automatically generates a date with format '%Y-%m-%d', while\n> asciidoctor '%F'.\n\nIs that really the format Asciidoc uses? Not that it matters much for\nthe purposes of this patch.\n\n> I personally prefer the latter, so only modify it for diff purposes.\n\nI agree completely.\n\n> Fixes tons of these:\n>\n> -Git omitted 01/01/1970 GIT-ADD(1)\n> +Git omitted 1970-01-01 GIT-ADD(1)\n\n> then\n> - echo USE_ASCIIDOCTOR=YesPlease\n> + echo USE_ASCIIDOCTOR=YesPlease ASCIIDOC_EXTRA='-adocdate=\"01/01/1970\"'\n> fi\n\nNice.\n\nIf you introduce a separate Makefile flag and incorporate Peff's patch\nto doc-diff \"asciidoctor\" and \"asciidoctor-direct\", you'd need to\nduplicate this a bit, or maybe just emit the ASCIIDOC_EXTRA outside of\nthe whole if chain.\n\nYou could follow up with the patch below. If you'd rather keep it out of\nyour series to avoid it ballooning, fine. I can repost it later, once\nthe dust has settled. Don't let it hold up your work.\n\n-- >8 --\nSubject: [PATCH] doc-diff: drop --cut-footer switch\n\nNow that our doc-diff convinces Asciidoctor to insert the exact same\nformatted dummy date as AsciiDoc, we can drop the --cut-footer switch.\nIt has been useful to ignore this difference between the two tools, but\nit's effectively a no-op now. Similar to when we repurposed this from\n--cut-header-footer in 83b0b8953e (\"doc-diff: replace\n--cut-header-footer with --cut-footer\", 2019-09-16), just drop it\nwithout worrying about any kind of backwards compatibility or user-base.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/doc-diff | 18 ++----------------\n 1 file changed, 2 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex aae5fc1933..97671ca65d 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -21,7 +21,6 @@ asciidoc\t\tuse asciidoc with both commits\n to-asciidoc\t\tuse asciidoc with the 'to'-commit\n to-asciidoctor\t\tuse asciidoctor with the 'to'-commit\n asciidoctor\t\tuse asciidoctor with both commits\n-cut-footer\t\tcut away footer\n \"\n SUBDIRECTORY_OK=1\n . \"$(git --exec-path)/git-sh-setup\"\n@@ -31,7 +30,6 @@ force=\n clean=\n from_program=\n to_program=\n-cut_footer=\n while test $# -gt 0\n do\n \tcase \"$1\" in\n@@ -55,8 +53,6 @@ do\n \t--asciidoc)\n \t\tfrom_program=-asciidoc\n \t\tto_program=-asciidoc ;;\n-\t--cut-footer)\n-\t\tcut_footer=-cut-footer ;;\n \t--)\n \t\tshift; break ;;\n \t*)\n@@ -118,8 +114,8 @@ construct_makemanflags () {\n from_makemanflags=$(construct_makemanflags \"$from_program\") &&\n to_makemanflags=$(construct_makemanflags \"$to_program\") &&\n \n-from_dir=$from_oid$from_program$cut_footer &&\n-to_dir=$to_oid$to_program$cut_footer &&\n+from_dir=$from_oid$from_program &&\n+to_dir=$to_oid$to_program &&\n \n # generate_render_makefile \n generate_render_makefile () {\n@@ -168,16 +164,6 @@ render_tree () {\n \t\t\t\"$tmp/rendered/$dname+\" |\n \t\tmake -j$parallel -f - &&\n \t\tmv \"$tmp/rendered/$dname+\" \"$tmp/rendered/$dname\"\n-\n-\t\tif test \"$cut_footer\" = \"-cut-footer\"\n-\t\tthen\n-\t\t\tfor f in $(find \"$tmp/rendered/$dname\" -type f)\n-\t\t\tdo\n-\t\t\t\thead -n -2 \"$f\" | sed -e '${/^$/d}' >\"$f+\" &&\n-\t\t\t\tmv \"$f+\" \"$f\" ||\n-\t\t\t\treturn 1\n-\t\t\tdone\n-\t\tfi\n \tfi\n }\n \n-- \n2.31.1.751.gd2f1c929bd\n\n"},{"id":"424621","messageId":"609edce68cff1_43127208aa@natae.notmuch","threadId":"55690","inReplyTo":"CAN0heSpX53tK8Z4XSx4sp79b+XWKZg5+ABW8pmzBSHPZ+qy+oQ@mail.gmail.com","subject":"Re: [PATCH 04/11] doc: use asciidoctor to build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T20:26:14Z","receivedAt":"2021-05-14T20:26:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Martin Ågren wrote:\n> On Fri, 14 May 2021 at 14:14, Felipe Contreras\n> wrote:\n> >\n> > There's no need to use xmlto to build the man pages when modern\n> > asciidoctor can do it by itself.\n> >\n> > This new mode will be active only when USE_ASCIIDOCTOR is set.\n> \n> May I suggest incorporating something more like brian's patch here [1],\n> so that there's a separate knob for this thing?\n\nSure. Do you agree with the name? (USE_ASCIIDOCTOR_MANPAGE)\n\n> The commit message is short on details and makes it sound like this is\n> it, we're done. But then there are several patches to fix up things.\n> Which is a good approach, so that this patch doesn't need to do several\n> things at once. This commit message could say something about it, e.g.,\n> \n> The doc-diff here [which doc-diff? see below] is a XYZ-line diff.\n> Large parts of this difference will be addressed in the following\n> patches.\n\nRight. I'll include that.\n\n> About the use of doc-diff: If this commit introduces a new knob rather\n> than taking over USE_ASCIIDOCTOR=Yes, the next patch could be Peff's\n> patch to doc-diff that compares the two asciidoctor approaches [2], and\n> then the next few patches could diff between them. That would get the\n> asciidoc-vs-asciidoctor differences out of the way, so you can focus on\n> asciidoctor-vs-asciidoctor.\n\nYou mean [1]. I think that belongs on the same patch. It's important\nthat if we do have a new switch, doc-diff is able to use it.\n\nHowever, I personally don't need such switch, I want to compare\nasciidoc-vs-asciidoctor-manpage wholesale.\n\nI want to see *all* the diffs.\n\n> With a separate knob, it would feel like a lot easier decision to take\n> something like this. There are over 11000 lines in the doc-diff after\n> applying your series, and there's the missing boldness for literals.\n> Maybe those differences are all great (I would be missing the bold\n> literals, though). If this series doesn't affect someone using\n> \"vanilla\" USE_ASCIIDOCTOR=Yes, we could let this thing cook in master\n> and work incrementally on top.\n\nI did notice the missing boldness for literals, and I know how to fix\nit. It's a small hack. I also noticed a few small rendering issues.\n\nBut from my point of view after my patches this is 98% done. Most of the\nremaining diffs are fine, for example:\n\n-GIT-CHECK-REF-FOR(1) Git Manual GIT-CHECK-REF-FOR(1)\n+GIT-CHECK-REF-FORMAT(1) Git Manual GIT-CHECK-REF-FORMAT(1)\n\nThat's clearly a bug in asciidoc+docbook. Others are things asciidoctor\nrenders better, and most are whitespace noise.\n\nCheers.\n\n[1] https://lore.kernel.org/git/YJt1%2FDO1cXNTRNxK@coredump.intra.peff.net/\n\n-- \nFelipe Contreras"},{"id":"424622","messageId":"609edeab20e55_43127208c4@natae.notmuch","threadId":"55690","inReplyTo":"20210514154316.810-1-martin.agren@gmail.com","subject":"Re: [PATCH 03/11] doc: doc-diff: set docdate manually","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T20:33:47Z","receivedAt":"2021-05-14T20:33:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Martin Ågren wrote:\n> On Fri, 14 May 2021 at 14:14, Felipe Contreras wrote:\n> >\n> > Asciidoc automatically generates a date with format '%Y-%m-%d', while\n> > asciidoctor '%F'.\n> \n> Is that really the format Asciidoc uses? Not that it matters much for\n> the purposes of this patch.\n\nYes:\n\nhttps://github.com/asciidoc-py/asciidoc-py/blob/main/asciidoc/asciidoc.py#L1189\n\n> > I personally prefer the latter, so only modify it for diff purposes.\n> \n> I agree completely.\n> \n> > Fixes tons of these:\n> >\n> > -Git omitted 01/01/1970 GIT-ADD(1)\n> > +Git omitted 1970-01-01 GIT-ADD(1)\n> \n> > then\n> > - echo USE_ASCIIDOCTOR=YesPlease\n> > + echo USE_ASCIIDOCTOR=YesPlease ASCIIDOC_EXTRA='-adocdate=\"01/01/1970\"'\n> > fi\n> \n> Nice.\n> \n> If you introduce a separate Makefile flag and incorporate Peff's patch\n> to doc-diff \"asciidoctor\" and \"asciidoctor-direct\", you'd need to\n> duplicate this a bit, or maybe just emit the ASCIIDOC_EXTRA outside of\n> the whole if chain.\n\nIndeed.\n\n> You could follow up with the patch below. If you'd rather keep it out of\n> your series to avoid it ballooning, fine. I can repost it later, once\n> the dust has settled. Don't let it hold up your work.\n\nI'll include it. Seems to fit the context.\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"424635","messageId":"YJ+VGKUa0Kk3BRWQ@coredump.intra.peff.net","threadId":"55690","inReplyTo":"20210514121435.504423-2-felipe.contreras@gmail.com","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-15T09:32:08Z","receivedAt":"2021-05-15T09:32:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 14, 2021 at 07:14:25AM -0500, Felipe Contreras wrote:\n\n> Without `override` all additions will be ignored by make.\n\nThat's true of all of our Makefile variables. Is there a particular\nreason to give this one special treatment?\n\n-Peff\n"},{"id":"424636","messageId":"YJ+WbMx+HTjbV2ZP@coredump.intra.peff.net","threadId":"55690","inReplyTo":"20210514121435.504423-3-felipe.contreras@gmail.com","subject":"Re: [PATCH 02/11] doc: doc-diff: allow more than one flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-15T09:37:48Z","receivedAt":"2021-05-15T09:37:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 14, 2021 at 07:14:26AM -0500, Felipe Contreras wrote:\n\n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/doc-diff | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/doc-diff b/Documentation/doc-diff\n> index 1694300e50..ecd88b0524 100755\n> --- a/Documentation/doc-diff\n> +++ b/Documentation/doc-diff\n> @@ -146,7 +146,7 @@ render_tree () {\n> \t# through.\n> \toid=$1 &&\n> \tdname=$2 &&\n> -\tmakemanflags=$3 &&\n> +\tmakemanflags=\"$3\" &&\n\nThis line does nothing; the shell won't split whitespace here either\nway.\n\n> @@ -181,6 +181,6 @@ render_tree () {\n> \tfi\n> }\n> \n> -render_tree $from_oid $from_dir $from_makemanflags &&\n> -render_tree $to_oid $to_dir $to_makemanflags &&\n> +render_tree $from_oid $from_dir \"$from_makemanflags\" &&\n> +render_tree $to_oid $to_dir \"$to_makemanflags\" &&\n\nThis part is necessary (and sufficient).\n\nIt would be nice to mention in the commit message why the use of\n$makemanflags in render_tree must remain unquoted (as I did in mine when\nI made the same change).\n\n-Peff\n"},{"id":"424637","messageId":"YJ+WzATLCM8SW2GH@coredump.intra.peff.net","threadId":"55690","inReplyTo":"YJ+VGKUa0Kk3BRWQ@coredump.intra.peff.net","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-15T09:39:24Z","receivedAt":"2021-05-15T09:39:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 15, 2021 at 05:32:08AM -0400, Jeff King wrote:\n\n> On Fri, May 14, 2021 at 07:14:25AM -0500, Felipe Contreras wrote:\n> \n> > Without `override` all additions will be ignored by make.\n> \n> That's true of all of our Makefile variables. Is there a particular\n> reason to give this one special treatment?\n\nTo go into further detail: usually we distinguish variables we use\ninternally from user-facing ones, and include the latter in the former.\nI see a later patch wants to start passing ASCIIDOC_EXTRA on the\ncommand-line, and we'd use two variables for that.\n\n-Peff\n"},{"id":"424654","messageId":"609fba7e89291_e173a208ce@natae.notmuch","threadId":"55690","inReplyTo":"YJ+WbMx+HTjbV2ZP@coredump.intra.peff.net","subject":"Re: [PATCH 02/11] doc: doc-diff: allow more than one flag","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T12:11:42Z","receivedAt":"2021-05-15T12:11:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Fri, May 14, 2021 at 07:14:26AM -0500, Felipe Contreras wrote:\n> \n> > Signed-off-by: Felipe Contreras \n> > ---\n> > Documentation/doc-diff | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/Documentation/doc-diff b/Documentation/doc-diff\n> > index 1694300e50..ecd88b0524 100755\n> > --- a/Documentation/doc-diff\n> > +++ b/Documentation/doc-diff\n> > @@ -146,7 +146,7 @@ render_tree () {\n> > \t# through.\n> > \toid=$1 &&\n> > \tdname=$2 &&\n> > -\tmakemanflags=$3 &&\n> > +\tmakemanflags=\"$3\" &&\n> \n> This line does nothing; the shell won't split whitespace here either\n> way.\n\nRight.\n\n> > @@ -181,6 +181,6 @@ render_tree () {\n> > \tfi\n> > }\n> > \n> > -render_tree $from_oid $from_dir $from_makemanflags &&\n> > -render_tree $to_oid $to_dir $to_makemanflags &&\n> > +render_tree $from_oid $from_dir \"$from_makemanflags\" &&\n> > +render_tree $to_oid $to_dir \"$to_makemanflags\" &&\n> \n> This part is necessary (and sufficient).\n> \n> It would be nice to mention in the commit message why the use of\n> $makemanflags in render_tree must remain unquoted (as I did in mine when\n> I made the same change).\n\nAhh, I didn't see exactly how you implemented it. Now I see it's very\nsimilar.\n\n-- \nFelipe Contreras\n"},{"id":"424655","messageId":"609fbafc73fcd_e173a208f4@natae.notmuch","threadId":"55690","inReplyTo":"YJ+WzATLCM8SW2GH@coredump.intra.peff.net","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T12:13:48Z","receivedAt":"2021-05-15T12:13:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Sat, May 15, 2021 at 05:32:08AM -0400, Jeff King wrote:\n> \n> > On Fri, May 14, 2021 at 07:14:25AM -0500, Felipe Contreras wrote:\n> > \n> > > Without `override` all additions will be ignored by make.\n> > \n> > That's true of all of our Makefile variables. Is there a particular\n> > reason to give this one special treatment?\n> \n> To go into further detail: usually we distinguish variables we use\n> internally from user-facing ones, and include the latter in the former.\n> I see a later patch wants to start passing ASCIIDOC_EXTRA on the\n> command-line, and we'd use two variables for that.\n\nWell, it's not exactly user-facing; it's only needed for doc-diff.\n\nWould TEST_ASCIIDOC_EXTRA make sense?\n\n-- \nFelipe Contreras\n"},{"id":"424718","messageId":"YKIwDTQI3sBXu+7W@coredump.intra.peff.net","threadId":"55690","inReplyTo":"609fbafc73fcd_e173a208f4@natae.notmuch","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-17T08:57:49Z","receivedAt":"2021-05-17T08:57:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 15, 2021 at 07:13:48AM -0500, Felipe Contreras wrote:\n\n> Jeff King wrote:\n> > On Sat, May 15, 2021 at 05:32:08AM -0400, Jeff King wrote:\n> > \n> > > On Fri, May 14, 2021 at 07:14:25AM -0500, Felipe Contreras wrote:\n> > > \n> > > > Without `override` all additions will be ignored by make.\n> > > \n> > > That's true of all of our Makefile variables. Is there a particular\n> > > reason to give this one special treatment?\n> > \n> > To go into further detail: usually we distinguish variables we use\n> > internally from user-facing ones, and include the latter in the former.\n> > I see a later patch wants to start passing ASCIIDOC_EXTRA on the\n> > command-line, and we'd use two variables for that.\n> \n> Well, it's not exactly user-facing; it's only needed for doc-diff.\n\nIt's meant for the caller of \"make\". Your proposed use is within\ndoc-diff, but any user running \"make ASCIIDOC_EXTRA=foo\" would see the\ndifferent behavior.\n\n> Would TEST_ASCIIDOC_EXTRA make sense?\n\nI'd probably call it ASCIIDOC_FLAGS (like we have CFLAGS and LDFLAGS\nthat are meant for users to inform us of extra flags they'd like\npassed).\n\nOf course that may not solve your problem in a sense; if you want\ndoc-diff to override it, then that might conflict with a theoretical\nASCIIDOC_FLAGS somebody set in their config.mak (but we really are in\nthe realm of hypothetical here).\n\n-Peff\n"},{"id":"424723","messageId":"60a24b25499c0_126a0520823@natae.notmuch","threadId":"55690","inReplyTo":"YKIwDTQI3sBXu+7W@coredump.intra.peff.net","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-17T10:53:25Z","receivedAt":"2021-05-17T10:53:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Sat, May 15, 2021 at 07:13:48AM -0500, Felipe Contreras wrote:\n> > Jeff King wrote:\n\n> > > To go into further detail: usually we distinguish variables we use\n> > > internally from user-facing ones, and include the latter in the former.\n> > > I see a later patch wants to start passing ASCIIDOC_EXTRA on the\n> > > command-line, and we'd use two variables for that.\n> > \n> > Well, it's not exactly user-facing; it's only needed for doc-diff.\n> \n> It's meant for the caller of \"make\". Your proposed use is within\n> doc-diff, but any user running \"make ASCIIDOC_EXTRA=foo\" would see the\n> different behavior.\n\nYeah, they would, but I don't think it would be wrong behavior.\n\n> > Would TEST_ASCIIDOC_EXTRA make sense?\n> \n> I'd probably call it ASCIIDOC_FLAGS (like we have CFLAGS and LDFLAGS\n> that are meant for users to inform us of extra flags they'd like\n> passed).\n\nRight, but Makefiles do override those, like:\n\n override CFLAGS += -fPIC\n\nOtherwise builds may fail.\n\n> Of course that may not solve your problem in a sense; if you want\n> doc-diff to override it, then that might conflict with a theoretical\n> ASCIIDOC_FLAGS somebody set in their config.mak (but we really are in\n> the realm of hypothetical here).\n\nSetting ASCIIDOC_FLAGS in config.mk would not override the\nuser-supplied flags any more than setting them in the Makefile (they are\nvirtually the same thing as one includes the other).\n\nIt's only if the user has `override ASCIIDOC_FLAGS` in config.mk that\nsuch a problem would arise. And that's really hypothetical.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"424724","messageId":"YKJV8HBYCA7hEQiX@coredump.intra.peff.net","threadId":"55690","inReplyTo":"60a24b25499c0_126a0520823@natae.notmuch","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-17T11:39:28Z","receivedAt":"2021-05-17T11:39:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 17, 2021 at 05:53:25AM -0500, Felipe Contreras wrote:\n\n> > It's meant for the caller of \"make\". Your proposed use is within\n> > doc-diff, but any user running \"make ASCIIDOC_EXTRA=foo\" would see the\n> > different behavior.\n> \n> Yeah, they would, but I don't think it would be wrong behavior.\n\nIt depends what they're trying to do. If they write:\n\n make ASCIIDOC_EXTRA=--one-extra-option\n\nthen they probably intend to to add to the options we set. If they\nwrite:\n\n make ASCIIDOC_EXTRA='-acompat-mode -atabsize=4 ...etc...'\n\nwith the intent of replicating the flags but changing or removing some\nelements, then it would no longer do what they want.\n\nI don't mean to imply one is more right than the other (I'd suspect even\nthat the override behavior is more likely to be what somebody wants).\nI'm mostly pointing out that this is unlike the rest of our Makefiles,\nwhich do not ever use override (and that the effect is visible to the\ncaller, depending on what they want to do).\n\n> > I'd probably call it ASCIIDOC_FLAGS (like we have CFLAGS and LDFLAGS\n> > that are meant for users to inform us of extra flags they'd like\n> > passed).\n> \n> Right, but Makefiles do override those, like:\n> \n> override CFLAGS += -fPIC\n> \n> Otherwise builds may fail.\n\nSome Makefiles do, but in this project we have not historically used\noverride. Instead, we provide defaults for things like CFLAGS, expect\nthe use to replace them if they like, and then aggregate them (along\nwith other internal variables) into things like ALL_CFLAGS.\n\n> > Of course that may not solve your problem in a sense; if you want\n> > doc-diff to override it, then that might conflict with a theoretical\n> > ASCIIDOC_FLAGS somebody set in their config.mak (but we really are in\n> > the realm of hypothetical here).\n> \n> Setting ASCIIDOC_FLAGS in config.mk would not override the\n> user-supplied flags any more than setting them in the Makefile (they are\n> virtually the same thing as one includes the other).\n> \n> It's only if the user has `override ASCIIDOC_FLAGS` in config.mk that\n> such a problem would arise. And that's really hypothetical.\n\nI mean that if your doc-diff runs:\n\n make USE_ASCIIDOCTOR=Yes ASCIIDOC_FLAGS=-adocdate=01/01/1970\n\nthen that will override anything the user put into config.mak. If they\nhad some option like:\n\n ASCIIDOC_FLAGS = --load-path=/some/special/directory\n\nthey need for asciidoctor to run correctly on their system, then things\nwould break for them. But we don't even have a user-facing\nASCIIDOC_FLAGS now, and nobody is asking for it, so it's pretty\nhypothetical (I'd guess somebody in this situation would just set\nASCIIDOC=\"asciidoctor --load-path=...\", and that already doesn't work\nwith doc-diff).\n\n-Peff\n"},{"id":"424765","messageId":"60a29edea3d13_12801e20873@natae.notmuch","threadId":"55690","inReplyTo":"YKJV8HBYCA7hEQiX@coredump.intra.peff.net","subject":"Re: [PATCH 01/11] doc: allow the user to provide ASCIIDOC_EXTRA","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-17T16:50:38Z","receivedAt":"2021-05-17T16:51:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Mon, May 17, 2021 at 05:53:25AM -0500, Felipe Contreras wrote:\n> \n> > > It's meant for the caller of \"make\". Your proposed use is within\n> > > doc-diff, but any user running \"make ASCIIDOC_EXTRA=foo\" would see the\n> > > different behavior.\n> > \n> > Yeah, they would, but I don't think it would be wrong behavior.\n> \n> It depends what they're trying to do. If they write:\n> \n> make ASCIIDOC_EXTRA=--one-extra-option\n> \n> then they probably intend to to add to the options we set. If they\n> write:\n> \n> make ASCIIDOC_EXTRA='-acompat-mode -atabsize=4 ...etc...'\n> \n> with the intent of replicating the flags but changing or removing some\n> elements, then it would no longer do what they want.\n> \n> I don't mean to imply one is more right than the other (I'd suspect even\n> that the override behavior is more likely to be what somebody wants).\n\nYeah, but I am implying that one is more right than the other.\n\n> I'm mostly pointing out that this is unlike the rest of our Makefiles,\n> which do not ever use override (and that the effect is visible to the\n> caller, depending on what they want to do).\n\nIt's used in the main Makefile, although in a different way.\n\nI see how it is not consistent with the rest of the Makefiles, but I\nwonder why it's not being used. It's rather useful.\n\n> > > I'd probably call it ASCIIDOC_FLAGS (like we have CFLAGS and LDFLAGS\n> > > that are meant for users to inform us of extra flags they'd like\n> > > passed).\n> > \n> > Right, but Makefiles do override those, like:\n> > \n> > override CFLAGS += -fPIC\n> > \n> > Otherwise builds may fail.\n> \n> Some Makefiles do, but in this project we have not historically used\n> override. Instead, we provide defaults for things like CFLAGS, expect\n> the use to replace them if they like, and then aggregate them (along\n> with other internal variables) into things like ALL_CFLAGS.\n\nI know, but status quo is not an argument.\n\nIf we always did things the way we've always done things there would\nnever be progress.\n\nI'm aruging there's no value in giving the user the opportunity to break\nthe build by doing `make BASIC_CFLAGS=`. Yes, it's more historically\nconsistent, that doesn't mean it's good.\n\n> > > Of course that may not solve your problem in a sense; if you want\n> > > doc-diff to override it, then that might conflict with a theoretical\n> > > ASCIIDOC_FLAGS somebody set in their config.mak (but we really are in\n> > > the realm of hypothetical here).\n> > \n> > Setting ASCIIDOC_FLAGS in config.mk would not override the\n> > user-supplied flags any more than setting them in the Makefile (they are\n> > virtually the same thing as one includes the other).\n> > \n> > It's only if the user has `override ASCIIDOC_FLAGS` in config.mk that\n> > such a problem would arise. And that's really hypothetical.\n> \n> I mean that if your doc-diff runs:\n> \n> make USE_ASCIIDOCTOR=Yes ASCIIDOC_FLAGS=-adocdate=01/01/1970\n> \n> then that will override anything the user put into config.mak. If they\n> had some option like:\n> \n> ASCIIDOC_FLAGS = --load-path=/some/special/directory\n> \n> they need for asciidoctor to run correctly on their system, then things\n> would break for them. But we don't even have a user-facing\n> ASCIIDOC_FLAGS now, and nobody is asking for it, so it's pretty\n> hypothetical (I'd guess somebody in this situation would just set\n> ASCIIDOC=\"asciidoctor --load-path=...\", and that already doesn't work\n> with doc-diff).\n\nExactly, so it's unclear how much value we get by talking about these.\n\nEither way, I don't feel very strongly about `override ASCIIDOC_EXTRA`.\nI think it's superior but ASCIIDOC_FLAGS requires less changes, so I'm\nfine with that.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}