On Tue, 2 Jun 2020 at 14:56, Jonathan Wakely <jwakely....@gmail.com> wrote:
>
> On Tue, 2 Jun 2020 at 14:16, Martin Liška <mli...@suse.cz> wrote:
> >
> > On 6/2/20 1:48 PM, Martin Liška wrote:
> > > I tend to this approach. Let me prepare a patch candidate for it.
> >
> > There's a patch for it. Can you please Jonathan take a look?
>
> Looks great, thanks!
>
> + if name not in wildcard_prefixes:
> + msg = 'unsupported wilcard prefix'
>
> Typo "wilcard"
>
> + if pattern not in used_patterns:
> + self.errors.append(Error('a file pattern not used in a
> patch',
> + pattern))
>
> If the script printed this error I don't think I'd know what it was
> complaining about. How about "pattern doesn't match any changed files"
> ?
>
> I find the existing error 'file not changed in a patch' to be
> suboptimal too. We're checking revisions (or commits), not patches.
Along those lines, here's an incomplete patch (tests aren't updated
yet, no commit log, your latest change isn't merged ) to revise the
error messages. I've tried to make them more consistent (e.g change
'file not changed in a patch' to 'unchanged file mentioned in a
ChangeLog' which mirrors the error for the opposite condition,
'changed file not mentioned in a ChangeLog').
I've also moved line numbers to the start of the line (like GCC's own
diagnostics) and not used the "line" argument of the Error constructor
for things that aren't line numbers. I've aimed to be consistent, so
that line numbers come at the start, pathnames and patterns come at
the end (and there's a space before them).
I also suggest changing 'additional author must prepend with tab and 4
spaces' to 'additional author must be indented with one tab and four
spaces'.
The intent is to improve the user experience, since for many people
who are new to git *any* error can cause confusion, so extra,
gcc-specific errors that we issue should aim to be easy to understand.
Do you like the direction?
diff --git a/contrib/gcc-changelog/git_commit.py
b/contrib/gcc-changelog/git_commit.py
index 069900d7cbf..453f0f61803 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -173,10 +173,9 @@ class Error:
self.line = line
def __repr__(self):
- s = self.message
if self.line:
- s += ':"%s"' % self.line
- return s
+ return '%d: %s' % (self.line, self.message)
+ return self.message
class ChangeLogEntry:
@@ -376,8 +375,8 @@ class GitCommit:
elif additional_author_regex.match(line):
m = additional_author_regex.match(line)
if len(m.group('spaces')) != 4:
- msg = 'additional author must prepend with tab ' \
- 'and 4 spaces'
+ msg = 'additional author must be indented with '\
+ ' one tab and four spaces'
self.errors.append(Error(msg, line))
else:
author_tuple = (m.group('name'), None)
@@ -437,15 +436,14 @@ class GitCommit:
m = star_prefix_regex.match(line)
if m:
if len(m.group('spaces')) != 1:
- err = Error('one space should follow asterisk',
- line)
- self.errors.append(err)
+ msg = 'one space should follow asterisk'
+ self.errors.append(Error(msg, line))
else:
last_entry.lines.append(line)
else:
if last_entry.is_empty:
msg = 'first line should start with a tab, ' \
- 'asterisk and space'
+ 'an asterisk and a space'
self.errors.append(Error(msg, line))
else:
last_entry.lines.append(line)
@@ -526,8 +524,8 @@ class GitCommit:
used_patterns = set()
for entry in self.changelog_entries:
if not entry.files:
- msg = 'ChangeLog must contain at least one file entry'
- self.errors.append(Error(msg, entry.folder))
+ msg = 'no files mentioned for ChangeLog in directory: "%s"'
+ self.errors.append(Error(msg % entry.folder))
assert not entry.folder.endswith('/')
for file in entry.files:
if not self.is_changelog_filename(file):
@@ -539,7 +537,8 @@ class GitCommit:
if not self.is_changelog_filename(x[0])]
changed_files = set(cand)
for file in sorted(mentioned_files - changed_files):
- self.errors.append(Error('file not changed in a patch', file))
+ msg = 'unchanged file mentioned in a ChangeLog: "%s"'
+ self.errors.append(Error(msg % file))
for file in sorted(changed_files - mentioned_files):
if not self.in_ignored_location(file):
if file in self.new_files:
@@ -575,7 +574,7 @@ class GitCommit:
if used_pattern:
used_patterns.add(used_pattern)
else:
- msg = 'changed file not mentioned in a ChangeLog'
+ msg = 'changed file not mentioned in a ChangeLog: "%s"'
self.errors.append(Error(msg, file))
for pattern in mentioned_patterns:
@@ -589,8 +588,9 @@ class GitCommit:
full_path = os.path.join(entry.folder, file)
changelog_location = self.get_changelog_by_path(full_path)
if changelog_location != entry.folder:
- msg = 'wrong ChangeLog location "%s", should be "%s"'
- err = Error(msg % (entry.folder, changelog_location), file)
+ msg = 'wrong ChangeLog location for "%s": "%s"'
+ path = os.path.join(changelog_location, file)
+ err = Error(msg % (path, entry.folder))
self.errors.append(err)
def to_changelog_entries(self, use_commit_ts=False):
