[PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

[PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

[Martin Ågren]

I've taught `doc-diff` a few new knobs to support usage like

  $ ./doc-diff --from-asciidoc --to-asciidoctor HEAD HEAD

and I don't think I've messed it up too badly in the process. I'm open
to the idea that this might not be interesting to a whole lot of people.
But I have made some progress on fixing up Asciidoctor *and* AsciiDoc
issues using this, and once the output of the above command is empty --
which might not be too far off -- it could be interesting to try and
keep it that way using a bit of automation around these switches.

While using/testing these patches, I've made some progress on the
rendering of the headers and footers in Asciidoctor [1], so the
`--cut-header-footer` switch that I'm adding in the final patch might
hopefully not be necessary for too long. But we'd still need a
`--cut-footer` switch -- at least I would, on my system [2]. If this
series is considered generally sane, I'd be happy to rework the final
patch to `--cut-header` if that's preferred.

Patch 1 has a minor purely-textual merge conflict with
ma/asciidoctor-fixes.

[1] https://public-inbox.org/git/20190317144747.2418514-1-martin.agren@gmail.com/

[2] After [1], the date in the footer is still formatted differently
    here. It might be a locale thing, and I tend to shy away from even
    trying to understand those. :-/

Martin Ågren (4):
  Doc: auto-detect changed build flags
  doc-diff: let `render_tree()` take an explicit directory name
  doc-diff: support diffing from/to AsciiDoc(tor)
  doc-diff: add `--cut-header-footer`

 Documentation/.gitignore |  1 +
 Documentation/Makefile   | 23 ++++++++---
 Documentation/doc-diff   | 86 +++++++++++++++++++++++++++++++++-------
 3 files changed, 90 insertions(+), 20 deletions(-)

-- 
2.21.0

[PATCH 1/4] Doc: auto-detect changed build flags

[Martin Ågren]

If you build the documentation switching between different options,
e.g., to build with both Asciidoc and Asciidoctor, you'll probably find
yourself running `make -C Documentation clean` either too often (wasting
time) or too rarely (getting mixed builds).

Track the flags we're using in the documentation build, similar to how
the main Makefile tracks CFLAGS and prefix flags. Track ASCIIDOC_COMMON
directly rather than its individual components -- that should make it
harder to forget to update the tracking if/when we modify the build
commands.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/.gitignore |  1 +
 Documentation/Makefile   | 23 +++++++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index 3ef54e0adb..bf2bf271b5 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -13,3 +13,4 @@ mergetools-*.txt
 manpage-base-url.xsl
 SubmittingPatches.txt
 tmp-doc-diff/
+GIT-ASCIIDOCFLAGS
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 26a2342bea..b534623012 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -331,6 +331,15 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
 		show_tool_names can_merge "* " || :' >mergetools-merge.txt && \
 	date >$@
 
+TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
+
+GIT-ASCIIDOCFLAGS: FORCE
+	@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \
+	    if test x"$$FLAGS" != x"`cat GIT-ASCIIDOCFLAGS 2>/dev/null`" ; then \
+		echo >&2 "    * new asciidoc flags"; \
+		echo "$$FLAGS" >GIT-ASCIIDOCFLAGS; \
+            fi
+
 clean:
 	$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
 	$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
@@ -340,13 +349,14 @@ clean:
 	$(RM) SubmittingPatches.txt
 	$(RM) $(cmds_txt) $(mergetools_txt) *.made
 	$(RM) manpage-base-url.xsl
+	$(RM) GIT-ASCIIDOCFLAGS
 
-$(MAN_HTML): %.html : %.txt asciidoc.conf
+$(MAN_HTML): %.html : %.txt asciidoc.conf GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
-$(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
+$(OBSOLETE_HTML): %.html : %.txto asciidoc.conf GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -o $@+ $< && \
 	mv $@+ $@
@@ -358,12 +368,12 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
-%.xml : %.txt asciidoc.conf
+%.xml : %.txt asciidoc.conf GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_XML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
-user-manual.xml: user-manual.txt user-manual.conf
+user-manual.xml: user-manual.txt user-manual.conf GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_XML) -d book -o $@+ $< && \
 	mv $@+ $@
@@ -373,7 +383,8 @@ technical/api-index.txt: technical/api-index-skel.txt \
 	$(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh
 
 technical/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
-$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt asciidoc.conf
+$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt \
+	asciidoc.conf GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt
 
 SubmittingPatches.txt: SubmittingPatches
@@ -430,7 +441,7 @@ $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 WEBDOC_DEST = /pub/software/scm/git/docs
 
 howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
-$(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt
+$(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	sed -e '1,/^$$/d' $< | \
 	$(TXT_TO_HTML) - >$@+ && \
-- 
2.21.0

[PATCH 2/4] doc-diff: let `render_tree()` take an explicit directory name

[Martin Ågren]

In `render_tree()`, `$1` is documented to be the commit-ish/oid and we
use it as that with `git checkout`, but we mostly use it to form the
name of various directories. To separate these concerns, and because we
are about to construct the directory names a bit differently, take two
distinct arguments instead.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 32c83dd26f..3e975d3c5d 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -79,6 +79,9 @@ then
 	ln -s "$dots/config.mak" "$tmp/worktree/config.mak"
 fi
 
+from_dir=$from_oid &&
+to_dir=$to_oid &&
+
 # generate_render_makefile <srcdir> <dstdir>
 generate_render_makefile () {
 	find "$1" -type f |
@@ -94,7 +97,7 @@ generate_render_makefile () {
 	done
 }
 
-# render_tree <committish_oid>
+# render_tree <committish_oid> <directory_name>
 render_tree () {
 	# Skip install-man entirely if we already have an installed directory.
 	# We can't rely on make here, since "install-man" unconditionally
@@ -102,28 +105,31 @@ render_tree () {
 	# we then can't rely on during the render step). We use "mv" to make
 	# sure we don't get confused by a previous run that failed partway
 	# through.
-	if ! test -d "$tmp/installed/$1"
+	oid=$1 &&
+	dname=$2 &&
+	if ! test -d "$tmp/installed/$dname"
 	then
-		git -C "$tmp/worktree" checkout --detach "$1" &&
+		git -C "$tmp/worktree" checkout --detach "$oid" &&
 		make -j$parallel -C "$tmp/worktree" \
 			GIT_VERSION=omitted \
 			SOURCE_DATE_EPOCH=0 \
-			DESTDIR="$tmp/installed/$1+" \
+			DESTDIR="$tmp/installed/$dname+" \
 			install-man &&
-		mv "$tmp/installed/$1+" "$tmp/installed/$1"
+		mv "$tmp/installed/$dname+" "$tmp/installed/$dname"
 	fi &&
 
 	# As with "installed" above, we skip the render if it's already been
 	# done.  So using make here is primarily just about running in
 	# parallel.
-	if ! test -d "$tmp/rendered/$1"
+	if ! test -d "$tmp/rendered/$dname"
 	then
-		generate_render_makefile "$tmp/installed/$1" "$tmp/rendered/$1+" |
+		generate_render_makefile "$tmp/installed/$dname" \
+			"$tmp/rendered/$dname+" |
 		make -j$parallel -f - &&
-		mv "$tmp/rendered/$1+" "$tmp/rendered/$1"
+		mv "$tmp/rendered/$dname+" "$tmp/rendered/$dname"
 	fi
 }
 
-render_tree $from_oid &&
-render_tree $to_oid &&
-git -C $tmp/rendered diff --no-index "$@" $from_oid $to_oid
+render_tree $from_oid $from_dir &&
+render_tree $to_oid $to_dir &&
+git -C $tmp/rendered diff --no-index "$@" $from_dir $to_dir
-- 
2.21.0

[PATCH 3/4] doc-diff: support diffing from/to AsciiDoc(tor)

[Martin Ågren]

Provide `--from-asciidoctor` and `--to-asciidoctor` to select that the
"from" resp. "to" commit should be built with Asciidoctor, and provide
an `--asciidoctor` shortcut for giving both. Similarly, provide
--{from-,to-,}asciidoc for explicitly selecting AsciiDoc.

Implement this using the USE_ASCIIDOCTOR flag. Let's not enforce a
default here, but instead just let the Makefile fall back on whatever is
in config.mak, so that `./doc-diff foo bar` without any of of these new
options behaves exactly like it did before this commit.

Encode the choice into the directory names of our "installed" and
"rendered" files, so that we can run `./doc-diff --from-asciidoc
--to-asciidoctor HEAD HEAD` without our two runs stomping on each other.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 53 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 8 deletions(-)

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 3e975d3c5d..36fc2307a7 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -12,9 +12,15 @@ OPTIONS_SPEC="\
 doc-diff [options] <from> <to> [-- <diff-options>]
 doc-diff (-c|--clean)
 --
-j=n	parallel argument to pass to make
-f	force rebuild; do not rely on cached results
-c,clean	cleanup temporary working files
+j=n			parallel argument to pass to make
+f			force rebuild; do not rely on cached results
+c,clean			cleanup temporary working files
+from-asciidoc		use asciidoc with the 'from'-commit
+from-asciidoctor	use asciidoctor with the 'from'-commit
+asciidoc		use asciidoc with both commits
+to-asciidoc		use asciidoc with the 'to'-commit
+to-asciidoctor		use asciidoctor with the 'to'-commit
+asciidoctor		use asciidoctor with both commits
 "
 SUBDIRECTORY_OK=1
 . "$(git --exec-path)/git-sh-setup"
@@ -22,6 +28,8 @@ SUBDIRECTORY_OK=1
 parallel=
 force=
 clean=
+from_program=
+to_program=
 while test $# -gt 0
 do
 	case "$1" in
@@ -31,6 +39,20 @@ do
 		clean=t ;;
 	-f)
 		force=t ;;
+	--from-asciidoctor)
+		from_program=-asciidoctor ;;
+	--to-asciidoctor)
+		to_program=-asciidoctor ;;
+	--asciidoctor)
+		from_program=-asciidoctor
+		to_program=-asciidoctor ;;
+	--from-asciidoc)
+		from_program=-asciidoc ;;
+	--to-asciidoc)
+		to_program=-asciidoc ;;
+	--asciidoc)
+		from_program=-asciidoc
+		to_program=-asciidoc ;;
 	--)
 		shift; break ;;
 	*)
@@ -79,8 +101,21 @@ then
 	ln -s "$dots/config.mak" "$tmp/worktree/config.mak"
 fi
 
-from_dir=$from_oid &&
-to_dir=$to_oid &&
+construct_makemanflags () {
+	if test "$1" = "-asciidoc"
+	then
+		echo USE_ASCIIDOCTOR=
+	elif test "$1" = "-asciidoctor"
+	then
+		echo USE_ASCIIDOCTOR=YesPlease
+	fi
+}
+
+from_makemanflags=$(construct_makemanflags "$from_program") &&
+to_makemanflags=$(construct_makemanflags "$to_program") &&
+
+from_dir=$from_oid$from_program &&
+to_dir=$to_oid$to_program &&
 
 # generate_render_makefile <srcdir> <dstdir>
 generate_render_makefile () {
@@ -97,7 +132,7 @@ generate_render_makefile () {
 	done
 }
 
-# render_tree <committish_oid> <directory_name>
+# render_tree <committish_oid> <directory_name> <makemanflags>
 render_tree () {
 	# Skip install-man entirely if we already have an installed directory.
 	# We can't rely on make here, since "install-man" unconditionally
@@ -107,10 +142,12 @@ render_tree () {
 	# through.
 	oid=$1 &&
 	dname=$2 &&
+	makemanflags=$3 &&
 	if ! test -d "$tmp/installed/$dname"
 	then
 		git -C "$tmp/worktree" checkout --detach "$oid" &&
 		make -j$parallel -C "$tmp/worktree" \
+			$makemanflags \
 			GIT_VERSION=omitted \
 			SOURCE_DATE_EPOCH=0 \
 			DESTDIR="$tmp/installed/$dname+" \
@@ -130,6 +167,6 @@ render_tree () {
 	fi
 }
 
-render_tree $from_oid $from_dir &&
-render_tree $to_oid $to_dir &&
+render_tree $from_oid $from_dir $from_makemanflags &&
+render_tree $to_oid $to_dir $to_makemanflags &&
 git -C $tmp/rendered diff --no-index "$@" $from_dir $to_dir
-- 
2.21.0

[PATCH 4/4] doc-diff: add `--cut-header-footer`

[Martin Ågren]

AsciiDoc and Asciidoctor do not agree on what to write in the header and
footer of each man-page, i.e., the very first and the very last line of
*.[157]. Those differences can certainly be interesting in their own
right, but they clutter the output of `./doc-diff --from-asciidoc
--to-asciidoctor HEAD HEAD` quite a bit since the diff contains some
10-15 lines of noise per file diffed.

Teach doc-diff to cut away the first two and last two lines, i.e., the
header/footer and the empty line immediately following/preceding it.
Because Asciidoctor uses an extra empty line compared to AsciiDoc,
remove one more line at each end of the file, but only if it's empty.

An alternative approach might be to pass down `--no-header-footer`,
which both AsciiDoc and Asciidoctor understand, but it has some
drawbacks. First of all, the result doesn't build -- `xmlto` stumbles on
the resulting xml since it has multiple root elements. Second, it cuts
too much -- dropping the header loses the synopsis, which would be
interesting to diff.

Like in the previous commit, encode this option into the directory name
of the "installed" and "rendered" files. Otherwise, we wouldn't be able
to trust that what we use out of that cache actually corresponds to the
options given for this run. (We could optimize this caching a little
since this flag doesn't affect the contents of "installed" at all, but
let's punt on that.)

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 36fc2307a7..3355be4798 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -21,6 +21,7 @@ asciidoc		use asciidoc with both commits
 to-asciidoc		use asciidoc with the 'to'-commit
 to-asciidoctor		use asciidoctor with the 'to'-commit
 asciidoctor		use asciidoctor with both commits
+cut-header-footer	cut away header and footer
 "
 SUBDIRECTORY_OK=1
 . "$(git --exec-path)/git-sh-setup"
@@ -30,6 +31,7 @@ force=
 clean=
 from_program=
 to_program=
+cut_header_footer=
 while test $# -gt 0
 do
 	case "$1" in
@@ -53,6 +55,8 @@ do
 	--asciidoc)
 		from_program=-asciidoc
 		to_program=-asciidoc ;;
+	--cut-header-footer)
+		cut_header_footer=-cut-header-footer ;;
 	--)
 		shift; break ;;
 	*)
@@ -114,8 +118,8 @@ construct_makemanflags () {
 from_makemanflags=$(construct_makemanflags "$from_program") &&
 to_makemanflags=$(construct_makemanflags "$to_program") &&
 
-from_dir=$from_oid$from_program &&
-to_dir=$to_oid$to_program &&
+from_dir=$from_oid$from_program$cut_header_footer &&
+to_dir=$to_oid$to_program$cut_header_footer &&
 
 # generate_render_makefile <srcdir> <dstdir>
 generate_render_makefile () {
@@ -164,6 +168,17 @@ render_tree () {
 			"$tmp/rendered/$dname+" |
 		make -j$parallel -f - &&
 		mv "$tmp/rendered/$dname+" "$tmp/rendered/$dname"
+
+		if test "$cut_header_footer" = "-cut-header-footer"
+		then
+			for f in $(find "$tmp/rendered/$dname" -type f)
+			do
+				tail -n +3 "$f" | head -n -2 |
+				sed -e '1{/^$/d}' -e '${/^$/d}' >"$f+" &&
+				mv "$f+" "$f" ||
+				return 1
+			done
+		fi
 	fi
 }
 
-- 
2.21.0

Re: [PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

[Junio C Hamano]

Martin Ågren <martin.agren@gmail.com> writes:

> I've taught `doc-diff` a few new knobs to support usage like
>
>   $ ./doc-diff --from-asciidoc --to-asciidoctor HEAD HEAD

Nice.

> But I have made some progress on fixing up Asciidoctor *and* AsciiDoc
> issues using this, and once the output of the above command is empty --
> which might not be too far off -- it could be interesting to try and
> keep it that way using a bit of automation around these switches.

That would be exciting.

Re: [PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

[Jeff King]

On Sun, Mar 17, 2019 at 07:35:59PM +0100, Martin Ågren wrote:

> I've taught `doc-diff` a few new knobs to support usage like
> 
>   $ ./doc-diff --from-asciidoc --to-asciidoctor HEAD HEAD
> 
> and I don't think I've messed it up too badly in the process. I'm open
> to the idea that this might not be interesting to a whole lot of people.
> But I have made some progress on fixing up Asciidoctor *and* AsciiDoc
> issues using this, and once the output of the above command is empty --
> which might not be too far off -- it could be interesting to try and
> keep it that way using a bit of automation around these switches.

Very nice. All the patches look good to me.

The "from" and "to" variants of the options are a little awkward; these
are really properties of the actual endpoints. It would be nice if we
had some fixed syntax that defined the whole state, like:

  ./doc-diff asciidoc:HEAD asciidoctor:HEAD^

or something. But I think as you introduce new options (like the
header/footer cutting) that syntax would get pretty unwieldy. So
probably the separate options is the best way forward.

> While using/testing these patches, I've made some progress on the
> rendering of the headers and footers in Asciidoctor [1], so the
> `--cut-header-footer` switch that I'm adding in the final patch might
> hopefully not be necessary for too long. But we'd still need a
> `--cut-footer` switch -- at least I would, on my system [2]. If this
> series is considered generally sane, I'd be happy to rework the final
> patch to `--cut-header` if that's preferred.

I think what's here is fine for now. This is our own internal script, so
if options become useless later on, we can always cull them.

> [2] After [1], the date in the footer is still formatted differently
>     here. It might be a locale thing, and I tend to shy away from even
>     trying to understand those. :-/

Yeah, mine too.

-Peff

Re: [PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

[Martin Ågren]

On Tue, 19 Mar 2019 at 04:14, Jeff King <peff@peff.net> wrote:
>
> On Sun, Mar 17, 2019 at 07:35:59PM +0100, Martin Ågren wrote:
>
> > I've taught `doc-diff` a few new knobs to support usage like
> >
> >   $ ./doc-diff --from-asciidoc --to-asciidoctor HEAD HEAD
>
> Very nice. All the patches look good to me.

Thank you.

> The "from" and "to" variants of the options are a little awkward; these
> are really properties of the actual endpoints. It would be nice if we
> had some fixed syntax that defined the whole state, like:
>
>   ./doc-diff asciidoc:HEAD asciidoctor:HEAD^

Right. I wasn't terribly impressed by the look of the "--from-foo
--to-bar" syntax, but at least I felt like I couldn't possibly mess up
the handling of it. As opposed to parsing substrings and whatnot.

> or something. But I think as you introduce new options (like the
> header/footer cutting) that syntax would get pretty unwieldy. So
> probably the separate options is the best way forward.

And this too. A few more orthogonal parameters on top and it would get
hairy pretty quickly. (At least I got the feeling it would.)

I think that at least --cut-header-footer is pretty useless to apply to
only one of the sides (you'd be asking for 4-6 lines to definitely
differ, for each and every rendered file), so this particular parameter
would perhaps not be useful to bake into your alternative syntax.

> > I'd be happy to rework the final
> > patch to `--cut-header` if that's preferred.
>
> I think what's here is fine for now. This is our own internal script, so
> if options become useless later on, we can always cull them.

That's true.

> > [2] After [1], the date in the footer is still formatted differently
> >     here. It might be a locale thing, and I tend to shy away from even
> >     trying to understand those. :-/
>
> Yeah, mine too.

Thanks for a data point. So at least it's not just me.


Martin