Skip to content

Commit d2c9908

Browse files
avargitster
avar
authored and
gitster
committed
doc lint: fix bugs in, simplify and improve lint script
The lint-gitlink.perl script added in ab81411 (ci: validate "linkgit:" in documentation, 2016-05-04) was more complex than it needed to be. It: - Was using File::Find to recursively find *.txt files in Documentation/, let's instead use the Makefile as a source of truth for *.txt files, and pass it down to the script. - We now don't lint linkgit:* in RelNotes/* or technical/*, which we shouldn't have been doing in the first place anyway. - When the doc-diff script was added in beb188e (add a script to diff rendered documentation, 2018-08-06) we started sometimes having a "git worktree" under Documentation/. This tree contains a full checkout of git.git, as a result the "lint" script would recurse into that, and lint any *.txt file found in that entire repository. In practice the only in-tree "linkgit" outside of the Documentation/ tree is contrib/contacts/git-contacts.txt and contrib/subtree/git-subtree.txt, so this wouldn't emit any errors Now we instead simply trust the Makefile to give us *.txt files. Since the Makefile also knows what sections each page should be in we don't have to open the files ourselves and try to parse that out. As a bonus this will also catch bugs with the section line in the files themselves being incorrect. The structure of the new script is mostly based on t/check-non-portable-shell.pl. As an added bonus it will also use pos() to print where the problems it finds are, e.g. given an issue like: diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt [...] and line numbers. git-cherry therefore detects when commits have been -"copied" by means of linkgit:git-cherry-pick[1], linkgit:git-am[1] or -linkgit:git-rebase[1]. +"copied" by means of linkgit:git-cherry-pick[2], linkgit:git-am[3] or +linkgit:git-rebase[4]. We'll now emit: git-cherry.txt:20: error: git-cherry-pick[2]: wrong section (should be 1), shown with 'HERE' below: git-cherry.txt:20: '"copied" by means of linkgit:git-cherry-pick[2]' <-- HERE git-cherry.txt:20: error: git-am[3]: wrong section (should be 1), shown with 'HERE' below: git-cherry.txt:20: '"copied" by means of linkgit:git-cherry-pick[2], linkgit:git-am[3]' <-- HERE git-cherry.txt:21: error: git-rebase[4]: wrong section (should be 1), shown with 'HERE' below: git-cherry.txt:21: 'linkgit:git-rebase[4]' <-- HERE Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 3951eeb commit d2c9908

File tree

2 files changed

+55
-57
lines changed

2 files changed

+55
-57
lines changed

‎Documentation/Makefile‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,11 @@ print-man1:
478478
@for i in $(MAN1_TXT); do echo $$i; done
479479

480480
lint-docs::
481-
$(QUIET_LINT)$(PERL_PATH) lint-gitlink.perl
481+
$(QUIET_LINT)$(PERL_PATH) lint-gitlink.perl \
482+
$(HOWTO_TXT) $(DOC_DEP_TXT) \
483+
--section=1 $(MAN1_TXT) \
484+
--section=5 $(MAN5_TXT) \
485+
--section=7 $(MAN7_TXT)
482486

483487
ifeq ($(wildcard po/Makefile),po/Makefile)
484488
doc-l10n install-l10n::

‎Documentation/lint-gitlink.perl‎

Lines changed: 50 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,72 +2,66 @@
22

33
use strict;
44
use warnings;
5-
use File::Find;
6-
use Getopt::Long;
75

8-
my $basedir = ".";
9-
GetOptions("basedir=s" => \$basedir)
10-
or die("Cannot parse command line arguments\n");
6+
# Parse arguments, a simple state machine for input like:
7+
#
8+
# howto/*.txt config/*.txt --section=1 git.txt git-add.txt [...] --to-lint git-add.txt a-file.txt [...]
9+
my %TXT;
10+
my %SECTION;
11+
my $section;
12+
my $lint_these = 0;
13+
for my $arg (@ARGV) {
14+
if (my ($sec) = $arg =~ /^--section=(\d+)$/s) {
15+
$section = $sec;
16+
next;
17+
}
1118

12-
my $found_errors = 0;
19+
my ($name) = $arg =~ /^(.*?)\.txt$/s;
20+
unless (defined $section) {
21+
$TXT{$name} = $arg;
22+
next;
23+
}
1324

14-
sub report {
15-
my ($where, $what, $error) = @_;
16-
print "$where: $error: $what\n";
17-
$found_errors = 1;
25+
$SECTION{$name} = $section;
1826
}
1927

20-
sub grab_section {
21-
my ($page) = @_;
22-
open my $fh, "<", "$basedir/$page.txt";
23-
my $firstline = <$fh>;
24-
chomp $firstline;
25-
close $fh;
26-
my ($section) = ($firstline =~ /.*\((\d)\)$/);
27-
return $section;
28+
my $exit_code = 0;
29+
sub report {
30+
my ($pos, $line, $target, $msg) = @_;
31+
substr($line, $pos) = "' <-- HERE";
32+
$line =~ s/^\s+//;
33+
print "$ARGV:$.: error: $target: $msg, shown with 'HERE' below:\n";
34+
print "$ARGV:$.:\t'$line\n";
35+
$exit_code = 1;
2836
}
2937

30-
sub lint {
31-
my ($file) = @_;
32-
open my $fh, "<", $file
33-
or return;
34-
while (<$fh>) {
35-
my $where = "$file:$.";
36-
while (s/linkgit:((.*?)\[(\d)\])//) {
37-
my ($target, $page, $section) = ($1, $2, $3);
38+
@ARGV = sort values %TXT;
39+
die "BUG: Nothing to process!" unless @ARGV;
40+
while (<>) {
41+
my $line = $_;
42+
while ($line =~ m/linkgit:((.*?)\[(\d)\])/g) {
43+
my $pos = pos $line;
44+
my ($target, $page, $section) = ($1, $2, $3);
3845

39-
# De-AsciiDoc
40-
$page =~ s/{litdd}/--/g;
46+
# De-AsciiDoc
47+
$page =~ s/{litdd}/--/g;
4148

42-
if ($page !~ /^git/) {
43-
report($where, $target, "nongit link");
44-
next;
45-
}
46-
if (! -f "$basedir/$page.txt") {
47-
report($where, $target, "no such source");
48-
next;
49-
}
50-
my $real_section = grab_section($page);
51-
if ($real_section != $section) {
52-
report($where, $target,
53-
"wrong section (should be $real_section)");
54-
next;
55-
}
49+
if (!exists $TXT{$page}) {
50+
report($pos, $line, $target, "link outside of our own docs");
51+
next;
52+
}
53+
if (!exists $SECTION{$page}) {
54+
report($pos, $line, $target, "link outside of our sectioned docs");
55+
next;
56+
}
57+
my $real_section = $SECTION{$page};
58+
if ($section != $SECTION{$page}) {
59+
report($pos, $line, $target, "wrong section (should be $real_section)");
60+
next;
5661
}
5762
}
58-
close $fh;
59-
}
60-
61-
sub lint_it {
62-
lint($File::Find::name) if -f && /\.txt$/;
63-
}
64-
65-
if (!@ARGV) {
66-
find({ wanted => \&lint_it, no_chdir => 1 }, $basedir);
67-
} else {
68-
for (@ARGV) {
69-
lint($_);
70-
}
63+
# this resets our $. for each file
64+
close ARGV if eof;
7165
}
7266

73-
exit $found_errors;
67+
exit $exit_code;

0 commit comments

Comments
 (0)