Messages in this thread | | | From | SeongJae Park <> | Subject | Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking | Date | Sun, 26 Jul 2020 17:36:01 +0200 |
| |
On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote:
> On Sun, 2020-07-26 at 09:45 +0200, SeongJae Park wrote: > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -721,6 +721,7 @@ sub read_word_corrections { > > my %deprecated_terms_fix; > > read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); > > my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; > > +my %deprecated_terms_reported = map { $_ => 1 } > > overly verbose naming and this doesn't need initialization here. > > > @@ -2975,13 +2976,16 @@ sub process { > > ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { > > while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { > > my $deprecated_term = $1; > > + last if (exists($deprecated_terms_reported{$deprecated_term})); > > next if (...) to check if multiple terms exists on the same line
Agreed on these comments, thanks!
> > > + $deprecated_terms_reported{$deprecated_term} = 1; > > + > > But this does need to be reset to empty when checking the next file
Hmm... I though you mean reporting same term multiple times too verbose... Did I misunderstand your point?
> > > my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; > > $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); > > $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); > > my $msg_level = \&WARN; > > $msg_level = \&CHK if ($file); > > if (&{$msg_level}("DEPRECATED_TERM", > > - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && > > + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && > > $fix) { > > $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; > > I think it simpler to avoid emitting this on existing files.
Agreed, it's much simpler. However, my concerns on excluding existing file checks are:
1. Avoiding existing file checks will still not stop warning patches mentioning existing deprecated terms. 2. If the term mistakenly comes in newly, it would be hard to check it later. 3. Some future deprecations of terms might be applied to existing uses, as 's/fuck/hug' did.
> > I do not want to encourage relatively inexperienced people > to run checkpatch and submit inappropriate patches.
Me, neither. But, I think providing more warnings and references is better for that. Experienced people would be able to easily ignore the false positives. Simply limiting checks could allow people submitting inappropriate patches intorducing new uses of deprecated terms.
Thanks, SeongJae Park
| |