Re: [PATCH v4 1/2] Documentation: fix linkgit references

Jeff King [off-list ref] writes:

Likewise, I think we could build the whole HTML source and then actually
just look for broken links in it. But that script would probably end up
looking similar to this one, with s/linkgit/href/. But it does more
directly measure what we want, which is that the rendered doc is usable;

I debated about this myself, but chose to inspect the source
material, as that approach is easier to give actionable lint output
to the user that points out the file:lineno to be corrected.

it would catch something like using "--" instead of "{litdd}".

That is true indeed.  With the "source" approach, that would indeed
be harder.

quoted

+#!/bin/sh
+
+git grep -l linkgit: Documentation/ |
+while read path
+do
+	perl -e '

Is it worth just making this a perl script, rather than a shell script
with a giant inline perl script? Perl is actually really good at doing
that "grep" as it reads the file. :)

OK.

-- >8 --
From: Junio C Hamano <redacted>
Date: Wed, 4 May 2016 11:48:06 -0700
Subject: [PATCH v2] ci: validate "gitlink:" in documentation

It is easy to add incorrect "linkgit:<page>[<section>]" references
to our documentation suite.  Catch these common classes of errors:

 * Referring to Documentation/<page>.txt that does not exist.

 * Referring to a <page> outside the Git suite.  In general, <page>
   must begin with "git".

 * Listing the manual <section> incorrectly.  The first line of the
   Documentation/<page>.txt must end with "(<section>)".

with a new script "ci/lint-gitlink".

Signed-off-by: Junio C Hamano <redacted>
---
 ci/lint-gitlink | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100755 ci/lint-gitlink

diff --git a/ci/lint-gitlink b/ci/lint-gitlink
new file mode 100755
index 0000000..bb73e89
--- /dev/null
+++ b/ci/lint-gitlink

@@ -0,0 +1,60 @@
+#!/usr/bin/perl
+
+use File::Find;
+
+my $found_errors = 0;
+
+sub report {
+	my ($where, $what, $error) = @_;
+	print "$where: $error: $what\n";
+	$found_errors = 1;
+}
+
+sub grab_section {
+	my ($page) = @_;
+	open my $fh, "<", "Documentation/$page.txt";
+	my $firstline = <$fh>;
+	chomp $firstline;
+	close $fh;
+	my ($section) = ($firstline =~ /.*\((\d)\)$/);
+	return $section;
+}
+
+sub lint {
+	my ($file) = @_;
+	open my $fh, "<", $file
+		or return;
+	while (<$fh>) {
+		my $where = "$file:$.";
+		while (s/linkgit:((.*?)\[(\d)\])//) {
+			my ($target, $page, $section) = ($1, $2, $3);
+
+			# De-AsciiDoc
+			$page =~ s/{litdd}/--/g;
+
+			if ($page !~ /^git/) {
+				report($where, $target, "nongit link");
+				next;
+			}
+			if (! -f "Documentation/$page.txt") {
+				report($where, $target, "no such source");
+				next;
+			}
+			$real_section = grab_section($page);
+			if ($real_section != $section) {
+				report($where, $target,
+					"wrong section (should be $real_section)");
+				next;
+			}
+		}
+	}
+	close $fh;
+}
+
+sub lint_it {
+	lint($File::Find::name) if -f;
+}
+
+find({ wanted => \&lint_it, no_chdir => 1 }, "Documentation");
+
+exit $found_errors;

-- 
2.8.2-498-g6350fe8

On Wed, May 04, 2016 at 12:57:31PM -0700, Junio C Hamano wrote:

quoted

Is it worth just making this a perl script, rather than a shell script
with a giant inline perl script? Perl is actually really good at doing
that "grep" as it reads the file. :)

OK.

Hmm. This new version uses File::Find:

+sub lint_it {
+	lint($File::Find::name) if -f;
+}
+
+find({ wanted => \&lint_it, no_chdir => 1 }, "Documentation");

That will inspect non-source files, too.

Would:

  open(my $files, '-|', qw(git ls-files));
  while (<$files>) {
    chomp;
    ...
  }

make sense? Or a simpler but non-streaming spelling:

  my @files = map { chomp; $_ } `git ls-files`;

Or just taking the list of files on the command line as your original
did, and feeding `ls-files` from the caller. That also lets you do
"link-gitlink git-foo.txt", etc.

-Peff

On Wed, May 4, 2016 at 10:06 PM, Jeff King [off-list ref] wrote:

Would:

  open(my $files, '-|', qw(git ls-files));
  while (<$files>) {
    chomp;
    ...
  }

make sense? Or a simpler but non-streaming spelling:

  my @files = map { chomp; $_ } `git ls-files`;

As a minor sidenote you can equivalently write that as:

    chomp(my @files = `git ls-files`);

I.e. chomp itself when given a list will chomp each item of the list.
So no need for a map.

On Wed, May 04, 2016 at 11:54:52PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

  my @files = map { chomp; $_ } `git ls-files`;

As a minor sidenote you can equivalently write that as:

    chomp(my @files = `git ls-files`);

I.e. chomp itself when given a list will chomp each item of the list.
So no need for a map.

Right, thanks, that is more readable. I use the map form reflexively
because I am often doing stuff like:

  my @list = do {
    open(my $fh, '<', $fn);
    map { chomp; $_ }
  };

(or replacing "do" with an actual function which is returning a
grotesque pipeline of grep/map, etc).

-Peff