Expand the file name match so that the match end can be used to limit path character substitution.

Expand the file name match so that the match end can be used to limit path character substitution.

BUG=none
TEST=Run on trybot-windows.txt, \base\ remains that way in the first warning.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67911

[Scott Byer, 2010-12-01 01:38:25]

Being part of the crowd; needed something fun to tweak at the end of a stressful
day...

[Evan Martin, 2010-12-01 02:44:38]

LGTM

[Scott Byer, 2010-12-01 18:11:15]

Take another look - I had this mostly done before I left last night; adds in
case correction for the path names.

On 2010/12/01 02:44:38, Evan Martin wrote:
> LGTM

[evanm, 2010-12-01 18:59:57]

+erg for more complex elisp review -- he's way better at it than I am

http://codereview.chromium.org/5298012/diff/4001/tools/emacs/trybot-windows.txt
File tools/emacs/trybot-windows.txt (right):

http://codereview.chromium.org/5298012/diff/4001/tools/emacs/trybot-windows.t...
tools/emacs/trybot-windows.txt:22:
e:\b\build\slave\win\build\src\third_party\webkit\javascriptcore\wtf\text\StringImpl.h(90)
: warning C4355: 'this' : used in \base\ member initializer list
I just now realized what you're doing here; it might be clearer to instead add
at the end of the error message something like:
 and here is a backslash for testing: \

[Evan Martin, 2010-12-01 22:00:49]

This LGTM again, I wouldn't wait on Elliot.  One q about backslashes.

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el
File tools/emacs/trybot.el (right):

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el#newco...
tools/emacs/trybot.el:58: (while (re-search-forward
"\\\(^.:\\\\.*\\\\src\\\\\\\)\\\(.*?\\\)[(:]" nil t)
Why three backslashes on these?

It seems you have:
input: \\\(
after lisp string literal interpretation:  \(
but you can achieve the latter with just "\\(" no?

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el#newco...
tools/emacs/trybot.el:59: (replace-match "" nil t nil 1)
It appears the "1" here means we don't replace the stuff after the first (...),
is that right?  Not sure if it deserves a comment or if this is old hat to elisp
hackers.

[Scott Byer, 2010-12-01 22:31:05]

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el
File tools/emacs/trybot.el (right):

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el#newco...
tools/emacs/trybot.el:58: (while (re-search-forward
"\\\(^.:\\\\.*\\\\src\\\\\\\)\\\(.*?\\\)[(:]" nil t)
On 2010/12/01 22:00:49, Evan Martin wrote:
> Why three backslashes on these?
> 
> It seems you have:
> input: \\\(
> after lisp string literal interpretation:  \(
> but you can achieve the latter with just "\\(" no?

Done - old, bad habit.

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el#newco...
tools/emacs/trybot.el:59: (replace-match "" nil t nil 1)
Easy enough to put the cursor on the function name and hit C-h f <ret> to get
the documentation for it, so I typically don't document parameter usage.

On 2010/12/01 22:00:49, Evan Martin wrote:
> It appears the "1" here means we don't replace the stuff after the first
(...),
> is that right?  Not sure if it deserves a comment or if this is old hat to
elisp
> hackers.

[Evan Martin, 2010-12-01 22:42:08]

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el
File tools/emacs/trybot.el (right):

http://codereview.chromium.org/5298012/diff/10001/tools/emacs/trybot.el#newco...
tools/emacs/trybot.el:59: (replace-match "" nil t nil 1)
On 2010/12/01 22:31:05, Scott Byer wrote:
> Easy enough to put the cursor on the function name and hit C-h f <ret> to get
> the documentation for it, so I typically don't document parameter usage.

Fair enough.  In this case, I was about to say 'you are clobbering the filename
because your above regex matches a bunch of text and here you replace it with
""', and the 1 is pretty subtle, but it doesn't matter much.  :)