Ediff regression

Ediff regression

[Chong Yidong]

Something in the following checkin broke patch highlighting:

2007-06-12  Michael Kifer  <kifer@cs.stonybrook.edu>
	
	* ediff-mult.el
	(ediff-meta-buffer-brief-message,ediff-meta-buffer-verbose-message):
	new variables.
	(ediff-meta-buffer-message): variable deleted.
	(ediff-verbose-help-enabled): new variable.
	(ediff-toggle-verbose-help-meta-buffer): new function.
	(ediff-redraw-directory-group-buffer): made aware of short/verbose
	message options
	
	* ediff-ptch.el (ediff-context-diff-label-regexp): better regexp.
	(ediff-fixup-patch-map): improved heuristic.

To see this, save the following diff to a file foo.patch and open it.
The body of the patch will be highlighted as though it were a header.
The problem did not exist before the checkin.

diff -c configure.in.\~1.452.\~ /home/cyd/emacs/configure.in
*** configure.in.~1.452.~	2007-06-12 13:07:07.000000000 -0400
--- configure.in	2007-06-13 13:20:27.000000000 -0400
***************
*** 1874,1880 ****
      case "${with_x_toolkit}" in
        athena | lucid ) USE_X_TOOLKIT=LUCID ;;
        motif ) USE_X_TOOLKIT=MOTIF ;;
- dnl      open-look ) USE_X_TOOLKIT=OPEN_LOOK ;;
        gtk ) with_gtk=yes
  dnl Dont set this for GTK.  A lot of tests below assumes Xt when
  dnl USE_X_TOOLKIT is set.
--- 1874,1879 ----

Re: Ediff regression

[Michael Kifer]

I am not sure what does ediff have to do with patch file highlighting, but
this patch indeed broke something in ediff itself. I fixed that just now.

> Something in the following checkin broke patch highlighting:
> 
> 2007-06-12  Michael Kifer  <kifer@cs.stonybrook.edu>
> 	
> 	* ediff-mult.el
> 	(ediff-meta-buffer-brief-message,ediff-meta-buffer-verbose-message):
> 	new variables.
> 	(ediff-meta-buffer-message): variable deleted.
> 	(ediff-verbose-help-enabled): new variable.
> 	(ediff-toggle-verbose-help-meta-buffer): new function.
> 	(ediff-redraw-directory-group-buffer): made aware of short/verbose
> 	message options
> 	
> 	* ediff-ptch.el (ediff-context-diff-label-regexp): better regexp.
> 	(ediff-fixup-patch-map): improved heuristic.
> 
> To see this, save the following diff to a file foo.patch and open it.
> The body of the patch will be highlighted as though it were a header.
> The problem did not exist before the checkin.
> 
> diff -c configure.in.\~1.452.\~ /home/cyd/emacs/configure.in
> *** configure.in.~1.452.~	2007-06-12 13:07:07.000000000 -0400
> --- configure.in	2007-06-13 13:20:27.000000000 -0400
> ***************
> *** 1874,1880 ****
>       case "${with_x_toolkit}" in
>         athena | lucid ) USE_X_TOOLKIT=LUCID ;;
>         motif ) USE_X_TOOLKIT=MOTIF ;;
> - dnl      open-look ) USE_X_TOOLKIT=OPEN_LOOK ;;
>         gtk ) with_gtk=yes
>   dnl Dont set this for GTK.  A lot of tests below assumes Xt when
>   dnl USE_X_TOOLKIT is set.
> --- 1874,1879 ----
> 

Re: Ediff regression

[Chong Yidong]

kifer@cs.sunysb.edu (Michael Kifer) writes:

>> Something in the following checkin broke patch highlighting:
>> 
>> 2007-06-12  Michael Kifer  <kifer@cs.stonybrook.edu>
>> 	
>> 	* ediff-mult.el
>>      ....
>> 	
>> 	* ediff-ptch.el (ediff-context-diff-label-regexp): better regexp.
>>      ...
>
> I am not sure what does ediff have to do with patch file highlighting, but
> this patch indeed broke something in ediff itself. I fixed that just now.

I think the error was introduced in diff-mode.el.  According to the
CVS logs, you changed diff-mode.el along with ediff during this batch
of checkins.  However, you failed to document your diff-mode.el change
in the ChangeLog.

Re: Ediff regression

[Stefan Monnier]

> I think the error was introduced in diff-mode.el.  According to the
> CVS logs, you changed diff-mode.el along with ediff during this batch
> of checkins.  However, you failed to document your diff-mode.el change
> in the ChangeLog.

I just fixed his last change.  Thanks for bringing it up.
Michael, could you document your change in the ChangeLog?


        Stefan

Re: Ediff regression

[Michael Kifer]

> > I think the error was introduced in diff-mode.el.  According to the
> > CVS logs, you changed diff-mode.el along with ediff during this batch
> > of checkins.  However, you failed to document your diff-mode.el change
> > in the ChangeLog.
> 
> I just fixed his last change.  Thanks for bringing it up.
> Michael, could you document your change in the ChangeLog?
> 
> 
>         Stefan

Sorry, I did not touch diff-mode.el intentionally. I see that the log
points the finger in my direction, but I do not know how it happened. :-(
The expression to which you changed diff-font-lock-keywords is similar to
one in ediff, but I do not know if it is what you wanted in
diff-mode.el. It seems ok, but if you are not sure then it is better to
revert to version 1.100. Sorry for the mixup.


	--michael  

Re: Ediff regression

[Stefan Monnier]

> Sorry, I did not touch diff-mode.el intentionally. I see that the log
> points the finger in my direction, but I do not know how it happened. :-(
> The expression to which you changed diff-font-lock-keywords is similar to
> one in ediff, but I do not know if it is what you wanted in
> diff-mode.el. It seems ok, but if you are not sure then it is better to
> revert to version 1.100. Sorry for the mixup.

You changed "\\S-" into "[^\t]" which was obviously meant to be "[^ \t]".
So I fixed the obvious.  I don't have much preference between the two, so
I left "[^ \t]" there.  Maybe if you could say why you preferred "[^ \t]"
over "\\S-" in ediff, I can make up a good reason to do the same change in
diff-mode.el ;-)?


        Stefan

Re: Ediff regression

[Michael Kifer]

> > Sorry, I did not touch diff-mode.el intentionally. I see that the log
> > points the finger in my direction, but I do not know how it happened. :-(
> > The expression to which you changed diff-font-lock-keywords is similar to
> > one in ediff, but I do not know if it is what you wanted in
> > diff-mode.el. It seems ok, but if you are not sure then it is better to
> > revert to version 1.100. Sorry for the mixup.
> 
> You changed "\\S-" into "[^\t]" which was obviously meant to be "[^ \t]".
> So I fixed the obvious.  I don't have much preference between the two, so
> I left "[^ \t]" there.  Maybe if you could say why you preferred "[^ \t]"
> over "\\S-" in ediff, I can make up a good reason to do the same change in
> diff-mode.el ;-)?

It was [^ \t] in ediff from the beginning.  I changed it to [^\t] in ediff
because I wanted to take care of files that have spaces in their names. But
this was wrong and I reverted the change back to [^ \t].

I am not sure how that same change got into diff-mode.el. Maybe I did
something mechanically and did not notice.


	--michael  

Re: Ediff regression

[Stefan Monnier]

> It was [^ \t] in ediff from the beginning.  I changed it to [^\t] in ediff
> because I wanted to take care of files that have spaces in their names.
> But this was wrong and I reverted the change back to [^ \t].

Oh well, never mind.

> I am not sure how that same change got into diff-mode.el.  Maybe I did
> something mechanically and did not notice.

I'll leave to historians to try and figure it out, centuries from now.


        Stefan