On 19/12/2013 09:08, Daniel Jasper wrote:
On Wed, Dec 18, 2013 at 10:34 PM, Alp Toker <[email protected]
<mailto:[email protected]>> wrote:
Author: alp
Date: Wed Dec 18 15:34:07 2013
New Revision: 197608
URL: http://llvm.org/viewvc/llvm-project?rev=197608&view=rev
Log:
clang-format-diff.py: fix -regex/-iregex matching
While debating the finer points of file extension matching, we
somehow missed
the bigger problem that the current code will match anything
starting with the
default or user-specified pattern (e.g. lit.site.cfg.in
<http://lit.site.cfg.in>).
Fix this by doing what find(1) does, implicitly wrapping the
pattern with ^$.
Modified:
cfe/trunk/tools/clang-format/clang-format-diff.py
Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=197608&r1=197607&r2=197608&view=diff
==============================================================================
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Wed Dec 18
15:34:07 2013
@@ -43,7 +43,7 @@ def main():
help='apply edits to files instead of
displaying a diff')
parser.add_argument('-p', metavar='NUM', default=0,
help='strip the smallest prefix containing
P slashes')
- parser.add_argument('-regex', metavar='PATTERN', default='',
+ parser.add_argument('-regex', metavar='PATTERN', default=None,
help='custom pattern selecting file paths
to reformat '
'(case sensitive, override -iregex)')
parser.add_argument('-iregex', metavar='PATTERN', default=
@@ -66,11 +66,11 @@ def main():
if filename == None:
continue
- if args.regex != '':
- if not re.match(args.regex, filename):
+ if args.regex is not None:
I am not opposed, but was there a particular reason to change '' to None?
Yes. The empty string '' maps to '^$' which matches nothing, in the same
way 'a' maps to '^a$' which matches only the path "a".
It's fine to permit it, or perhaps disallow it as find(1) does, but
special-casing empty string to match everything was the one behaviour
that didn't add up.
I noticed this a couple of days when everything in my diff got formatted
instead of just matching no files (with arguments passed in from a script).
+ if not re.match('^%s$' % args.regex, filename):
AFAIK, re.match() only matches if the regex matches from the beginning
of the string. So, the "^" is unnecessary. The "$" is a good idea,
though :-).
Using both ^ and $ serves an important purpose here -- namely, the three
of us managed to consistently miss what was going on here. Writing out
both start and end matchers explicitly makes it much clearer.
Alp.
continue
else:
- if not re.match(args.iregex, filename, re.IGNORECASE):
+ if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
continue
match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
_______________________________________________
cfe-commits mailing list
[email protected] <mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
