vc-set-mode-line-busy-indicator

vc-set-mode-line-busy-indicator

[Nick Roberts]

The problem with this message is that it is so long that the bit after ("from
filename") doesn't fit in 80 columns.  Also a warning-face isn't really needed
because it doesn't provide a wake up call: the user has just entered a command,
e.g, vc-diff or vc-print-log, so presumably he's already looking for the output
buffer.

I've used the same style as I've used for the GUD buffer.  If text in the
mode-line is going to be increasingly fontified, it might be a good idea to
have some consistency.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** vc.el.~1.541.~	2008-02-24 08:24:18.000000000 +1300
--- vc.el	2008-02-26 01:29:24.000000000 +1300
*************** BUF defaults to \"*vc*\", can be a strin
*** 1064,1077 ****
  
  (defun vc-set-mode-line-busy-indicator ()
    (setq mode-line-process
! 	;; Deliberate overstatement, but power law respected.
! 	;; (The message is ephemeral, so we make it loud.)  --ttn
! 	(propertize " (incomplete/in progress)"
! 		    'face (if (featurep 'compile)
! 			      ;; ttn's preferred loudness
! 			      'compilation-warning
! 			    ;; suitably available fallback
! 			    font-lock-warning-face)
  		    'help-echo
  		    "A VC command is in progress in this buffer")))
  
--- 1064,1071 ----
  
  (defun vc-set-mode-line-busy-indicator ()
    (setq mode-line-process
! 	(propertize " [waiting...]"
! 		    'face 'font-lock-variable-name-face
  		    'help-echo
  		    "A VC command is in progress in this buffer")))
  

Re: vc-set-mode-line-busy-indicator

[Nick Roberts]

Nick Roberts writes:
 > 
 > The problem with this message is that it is so long that the bit after
 > ("from filename") doesn't fit in 80 columns.  Also a warning-face isn't
 > really needed because it doesn't provide a wake up call: the user has just
 > entered a command, e.g, vc-diff or vc-print-log, so presumably he's already
 > looking for the output buffer.
 > 
 > I've used the same style as I've used for the GUD buffer.  If text in the
 > mode-line is going to be increasingly fontified, it might be a good idea to
 > have some consistency.

I forgot to ask:

Shall I apply this patch?

 > -- 
 > Nick                                           http://www.inet.net.nz/~nickrob
 > 
 > 
 > *** vc.el.~1.541.~	2008-02-24 08:24:18.000000000 +1300
 > --- vc.el	2008-02-26 01:29:24.000000000 +1300
 > *************** BUF defaults to \"*vc*\", can be a strin
 > *** 1064,1077 ****
 >   
 >   (defun vc-set-mode-line-busy-indicator ()
 >     (setq mode-line-process
 > ! 	;; Deliberate overstatement, but power law respected.
 > ! 	;; (The message is ephemeral, so we make it loud.)  --ttn
 > ! 	(propertize " (incomplete/in progress)"
 > ! 		    'face (if (featurep 'compile)
 > ! 			      ;; ttn's preferred loudness
 > ! 			      'compilation-warning
 > ! 			    ;; suitably available fallback
 > ! 			    font-lock-warning-face)
 >   		    'help-echo
 >   		    "A VC command is in progress in this buffer")))
 >   
 > --- 1064,1071 ----
 >   
 >   (defun vc-set-mode-line-busy-indicator ()
 >     (setq mode-line-process
 > ! 	(propertize " [waiting...]"
 > ! 		    'face 'font-lock-variable-name-face
 >   		    'help-echo
 >   		    "A VC command is in progress in this buffer")))
 >   

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Nick Roberts wrote:

>  > ! 	(propertize " [waiting...]"
>  > ! 		    'face 'font-lock-variable-name-face

I much prefer your wording.

With regards to the face, any chance of a new one? I know this is what
people always say, and that for some reason creating new faces is
sometimes frowned upon. The practical reason is, the mode-line often
has a different background colour to the main buffer, so a font-lock
face customized to look good in the buffer may look poor on the
mode-line. Eg for me, font-lock-warning-face is practically invisible
in the mode-line.

Re: vc-set-mode-line-busy-indicator

[Nick Roberts]

 > >  > ! 	(propertize " [waiting...]"
 > >  > ! 		    'face 'font-lock-variable-name-face
 > 
 > I much prefer your wording.
 > 
 > With regards to the face, any chance of a new one? I know this is what
 > people always say, and that for some reason creating new faces is
 > sometimes frowned upon. The practical reason is, the mode-line often
 > has a different background colour to the main buffer, so a font-lock
 > face customized to look good in the buffer may look poor on the
 > mode-line. Eg for me, font-lock-warning-face is practically invisible
 > in the mode-line.

I don't know enough to suggest a better one.  Perhaps someone else can.  I just
selected a quiet font-lock face, as font-lock faces are always available.

On a related note, I think that adding colour to the mode-line of the already
psychedelic compilation buffer means ironically that the plain text starts to
stand out.  Bright colours only draw attention to a part of the screen if they
are used in moderation.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Nick Roberts wrote:

>  > >  > ! 	(propertize " [waiting...]"
>  > >  > ! 		    'face 'font-lock-variable-name-face

Sorry, but now this is installed I see it is totally invisible on my
mode-line.

Please can I either remove the 'face altogether or create a new face
for it. I don't care which.

Re: vc-set-mode-line-busy-indicator

[Nick Roberts]

 > >  > >  > ! 	(propertize " [waiting...]"
 > >  > >  > ! 		    'face 'font-lock-variable-name-face
 > 
 > Sorry, but now this is installed I see it is totally invisible on my
 > mode-line.
 > 
 > Please can I either remove the 'face altogether or create a new face
 > for it. I don't care which.

I committed it because that seemed to fit the culture being advocated.  If it
doesn't work for you then please do change it accordingly.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Nick Roberts wrote:

>  > Sorry, but now this is installed I see it is totally invisible on my
>  > mode-line.
>  > 
>  > Please can I either remove the 'face altogether or create a new face
>  > for it. I don't care which.
>
> I committed it because that seemed to fit the culture being advocated.  If it
> doesn't work for you then please do change it accordingly.

I don't know nothing' 'bout no culture...

I should have tried it out to see how it looked.

If people feel the need to emphasize certain mode-line features (and
if you did a similar thing for gud, there probably is such a need),
then I would say it needs a new face; because the mode-line palette is
different to the buffer palette.

I suggest a new `mode-line-emphasis' face. The existing
`mode-line-highlight' doesn't look right because it is designed for
buttons. I made up some defaults; I'm not sure about them.


*** faces.el.~1.394.~	2008-02-23 11:24:00.000000000 -0800
--- faces.el	2008-03-01 14:47:47.000000000 -0800
***************
*** 2388,2398 ****
       :box (:line-width 2 :color "grey40" :style released-button))
      (t
       :inherit highlight))
!   "Basic mode line face for highlighting."
    :version "22.1"
    :group 'mode-line-faces
    :group 'basic-faces)
  
  (defface mode-line-buffer-id
    '((t (:weight bold)))
    "Face used for buffer identification parts of the mode line."
--- 2388,2409 ----
       :box (:line-width 2 :color "grey40" :style released-button))
      (t
       :inherit highlight))
!   "Basic mode line face for highlighting (of buttons, etc)."
    :version "22.1"
    :group 'mode-line-faces
    :group 'basic-faces)
  
+ (defface mode-line-emphasis
+   '((((class color) (min-colors 88))
+      :background "grey40")
+     (t
+      :inherit highlight))
+   "Face used to emphasize certain mode line features.
+ Use the face `mode-line-highlight' for features that can be selected."
+   :version "23.1"
+   :group 'mode-line-faces
+   :group 'basic-faces)
+ 
  (defface mode-line-buffer-id
    '((t (:weight bold)))
    "Face used for buffer identification parts of the mode line."


*** vc.el.~1.545.~	2008-03-01 11:41:50.000000000 -0800
--- vc.el	2008-03-01 14:43:10.000000000 -0800
***************
*** 1063,1072 ****
  
  (defun vc-set-mode-line-busy-indicator ()
    (setq mode-line-process
! 	(propertize " [waiting...]"
! 		    'face 'font-lock-variable-name-face
  		    'help-echo
! 		    "A VC command is in progress in this buffer")))
  
  (defun vc-exec-after (code)
    "Eval CODE when the current buffer's process is done.
--- 1063,1072 ----
  
  (defun vc-set-mode-line-busy-indicator ()
    (setq mode-line-process
! 	(concat " " (propertize "[waiting...]"
!                                 'face 'mode-line-emphasis
                                  'help-echo
!                                 "A VC command is in progress in this buffer"))))
  
  (defun vc-exec-after (code)
    "Eval CODE when the current buffer's process is done.

Re: vc-set-mode-line-busy-indicator

[Juri Linkov]

> If people feel the need to emphasize certain mode-line features (and
> if you did a similar thing for gud, there probably is such a need),
> then I would say it needs a new face; because the mode-line palette is
> different to the buffer palette.
>
> I suggest a new `mode-line-emphasis' face. The existing
> `mode-line-highlight' doesn't look right because it is designed for
> buttons. I made up some defaults; I'm not sure about them.
> [...]
> + (defface mode-line-emphasis
> +   '((((class color) (min-colors 88))
> +      :background "grey40")
> +     (t
> +      :inherit highlight))
> +   "Face used to emphasize certain mode line features.
> + Use the face `mode-line-highlight' for features that can be selected."
> +   :version "23.1"
> +   :group 'mode-line-faces
> +   :group 'basic-faces)

Text with black foreground is not very readable on dark background grey40.
On the mode line there are already places where text is emphasized: buffer
names are displayed in bold (bold is also often used to make *emphasis* in
printed materials).  Isn't bold strong enough to emphasize the "[waiting...]"
string?  Then we could add a new general bold face `mode-line-emphasis'
and change `mode-line-buffer-id' to inherit from it.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Juri Linkov wrote:

> On the mode line there are already places where text is emphasized: buffer
> names are displayed in bold (bold is also often used to make *emphasis* in
> printed materials).  Isn't bold strong enough to emphasize the "[waiting...]"
> string?  Then we could add a new general bold face `mode-line-emphasis'
> and change `mode-line-buffer-id' to inherit from it.

I'd have no problem with the default emphasis face being bold.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Glenn Morris <rgm@gnu.org> writes:

  > Juri Linkov wrote:
  > 
  > > On the mode line there are already places where text is emphasized: buffer
  > > names are displayed in bold (bold is also often used to make *emphasis* in
  > > printed materials).  Isn't bold strong enough to emphasize the "[waiting...]"
  > > string?  Then we could add a new general bold face `mode-line-emphasis'
  > > and change `mode-line-buffer-id' to inherit from it.
  > 
  > I'd have no problem with the default emphasis face being bold.

If it's just bold and no other colors, then it's not a good idea.  For
some fonts the bold form is not easy to distinguish from the normal
one.  More, bold is hard to see with the peripheral vision, so changes
in the mode-line are harder to observe.

Re: vc-set-mode-line-busy-indicator

[Nick Roberts]

Dan Nicolaescu writes:
 > Glenn Morris <rgm@gnu.org> writes:
 > 
 >   > Juri Linkov wrote:
 >   > 
 >   > > On the mode line there are already places where text is emphasized: buffer
 >   > > names are displayed in bold (bold is also often used to make *emphasis* in
 >   > > printed materials).  Isn't bold strong enough to emphasize the "[waiting...]"
 >   > > string?  Then we could add a new general bold face `mode-line-emphasis'
 >   > > and change `mode-line-buffer-id' to inherit from it.
 >   > 
 >   > I'd have no problem with the default emphasis face being bold.
 > 
 > If it's just bold and no other colors, then it's not a good idea.  For
 > some fonts the bold form is not easy to distinguish from the normal
 > one.  More, bold is hard to see with the peripheral vision, so changes
 > in the mode-line are harder to observe.

For now, I suggest we just go with Juri's idea.  I'm sure it's no good for
users with tunnel vision and without bold fonts, but it seems an improvement
and probably covers most users.

Later, if the need arises, someone can build a framework of faces that work on
the mode-line.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: vc-set-mode-line-busy-indicator

[Juri Linkov]

>  > If it's just bold and no other colors, then it's not a good idea.  For
>  > some fonts the bold form is not easy to distinguish from the normal
>  > one.  More, bold is hard to see with the peripheral vision, so changes
>  > in the mode-line are harder to observe.
>
> For now, I suggest we just go with Juri's idea.  I'm sure it's no good for
> users with tunnel vision and without bold fonts, but it seems an improvement
> and probably covers most users.
>
> Later, if the need arises, someone can build a framework of faces that
> work on the mode-line.

Please note that my opinion was mostly about the face name:
in typography emphasis is usually achieved by bold (and italic).
If the need for more strong indication arises then a better
name for the face with stronger colors would be `mode-line-warning'.

Since now Richard argued even for using no mode line indication at all,
bold doesn't seem too bad.  It is not strong and still helps to notice
the status of the vc command.

For fonts that don't support bold we could use the defface's keyword
`supports' to detect this and use a fallback definition.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    I suggest a new `mode-line-emphasis' face. The existing
    `mode-line-highlight' doesn't look right because it is designed for
    buttons. I made up some defaults; I'm not sure about them.

Does this proposal mean having two different faces for two different
kinds of highlighting in the mode line?

What sort of VC commands are asynchronous, other than vc-diff which
shouldn't be a problem?

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  > What sort of VC commands are asynchronous, other than vc-diff which
  > shouldn't be a problem?

All non-state changing VC commands should be asynchronous. Currently
vc-diff, vc-annotate, vc-log and vc-status are.

Not sure what you mean above about vc-diff... 
vc-diff is essential to be asynchronous, computing diffs can take
several minutes (see what Tom Tromey said in another thread about GCC
svn).  Having an indication in the mode-line that the diff is still
active is very good, especially if the user has selected a large number
a files for diffing.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

      > What sort of VC commands are asynchronous, other than vc-diff which
      > shouldn't be a problem?

    All non-state changing VC commands should be asynchronous. Currently
    vc-diff, vc-annotate, vc-log and vc-status are.

    Not sure what you mean above about vc-diff... 

I see no need to inform people in the mode line that vc-diff is
running, or that it has stopped.  Likewise vc-log.  vc-status must be
something new; I don't know what it is.  But any asynch vc command
whose purpose is to display something need not put anything in the
mode line.  Better to work like vc-diff: make the output buffer
appear, and the user will see when it is complete.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >       > What sort of VC commands are asynchronous, other than vc-diff which
  >       > shouldn't be a problem?
  > 
  >     All non-state changing VC commands should be asynchronous. Currently
  >     vc-diff, vc-annotate, vc-log and vc-status are.
  > 
  >     Not sure what you mean above about vc-diff... 
  > 
  > I see no need to inform people in the mode line that vc-diff is
  > running, or that it has stopped. 
  > 
  >  Likewise vc-log.  vc-status must be
  > something new; I don't know what it is.  But any asynch vc command
  > whose purpose is to display something need not put anything in the
  > mode line.  Better to work like vc-diff: make the output buffer
  > appear, and the user will see when it is complete.

vc-diff was one of the reasons for adding the mode-line notification:
the user can't see that the output is complete.  vc-diff operations can
take a long time.  Using CVS with a busy server/slow network tends to
have very non-predictable behavior, one can't see when the diff
operations has finished.  If the user actually wants to use the result
of vc-diff, for example to email it to someone else, then he can get
incomplete results.  The mode-line notification is a very good way to
show that the VC command is still in progress. 

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    vc-diff was one of the reasons for adding the mode-line notification:
    the user can't see that the output is complete.

It moves point when it completes, so that you can tell.
Until then, point remains at the top.

If you start moving point before it completes, then
maybe you can't tell, unless you move to the end.

Are we talking about the mode line of that buffer only?
That might be reasonable.  I thought people were talking
about mode lines of all buffers.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >     vc-diff was one of the reasons for adding the mode-line notification:
  >     the user can't see that the output is complete.
  > 
  > It moves point when it completes, so that you can tell.
  > Until then, point remains at the top.
  > 
  > If you start moving point before it completes, then
  > maybe you can't tell, unless you move to the end.

Even if you move to the end, you can't tell if the command is done 

  > Are we talking about the mode line of that buffer only?

We are talking about vc-diff, vc-annotate, vc-log buffers only.

  > That might be reasonable.  I thought people were talking about mode
  > lines of all buffers.

No, it's not for all buffers, just for buffers that VC uses to execute
commands that can take a long time.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

      > If you start moving point before it completes, then
      > maybe you can't tell, unless you move to the end.

    Even if you move to the end, you can't tell if the command is done 

Why not?  Isn't there some text that appears at the end when the command
is finished?  If not, let's add some.

      > That might be reasonable.  I thought people were talking about mode
      > lines of all buffers.

    No, it's not for all buffers, just for buffers that VC uses to execute
    commands that can take a long time.

I have no objection to that, but adding text at the end when it finishes
also seems like a good idea.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >       > If you start moving point before it completes, then
  >       > maybe you can't tell, unless you move to the end.
  > 
  >     Even if you move to the end, you can't tell if the command is done 
  > 
  > Why not?  Isn't there some text that appears at the end when the
  > command is finished?

No, there isn't.

  > If not, let's add some.

Please don't, there temporarily was some for a few months, and it was
discussed at length on the list and voted out.  It is annoying when you
have to copy the buffer that there's extra text that is not part of the
diff that you have to deal with.


  >       > That might be reasonable.  I thought people were talking about mode
  >       > lines of all buffers.
  > 
  >     No, it's not for all buffers, just for buffers that VC uses to execute
  >     commands that can take a long time.
  > 
  > I have no objection to that, but adding text at the end when it finishes
  > also seems like a good idea.

Please don't, see above.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    Please don't, there temporarily was some for a few months, and it was
    discussed at length on the list and voted out.  It is annoying when you
    have to copy the buffer that there's extra text that is not part of the
    diff that you have to deal with.

Suppose it says "End of diff output".  Why is it a problem if you copy
that along with the diff?  If you copy that, what harm does it do?
It won't bother `patch'.

Re: vc-set-mode-line-busy-indicator

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
> Suppose it says "End of diff output".  Why is it a problem if you copy
> that along with the diff?  If you copy that, what harm does it do?
> It won't bother `patch'.

I dunno, maybe it's just a personal foible, but emacs the "diff" command
used to add this sort of terminator line (haven't noticed recently) sort
of terminator string, and I always found it very annoying -- it probably
doesn't cause anything to _break_ but it was like an ugly little wart,
and I always ended up going to extra effort to delete it when saving the
output.

I think the highlighted mode-line status indicators used by
e.g. compile-mode (running or finished etc) now are really nice and do a
great job of quickly making me aware of what's going on.  I would love
it if asynchronous vc commands used a similar indicator to show their
status.

-Miles

-- 
"Though they may have different meanings, the cries of 'Yeeeee-haw!' and
 'Allahu akbar!' are, in spirit, not actually all that different."

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Miles Bader <miles.bader@necel.com> writes:

  > I think the highlighted mode-line status indicators used by
  > e.g. compile-mode (running or finished etc) now are really nice and do a
  > great job of quickly making me aware of what's going on.  

Thanks!

  > I would love it if asynchronous vc commands used a similar indicator
  > to show their status.

They should currently show "[waiting...]".  If you can think of
something better, please fix the message.  If the asynchronous commands
don't show this message, then it's a bug that needs fixing, so please
say when you find such a case.

Re: vc-set-mode-line-busy-indicator

[Jan Djärv]

Dan Nicolaescu skrev:
> Miles Bader <miles.bader@necel.com> writes:
> 
>   > I think the highlighted mode-line status indicators used by
>   > e.g. compile-mode (running or finished etc) now are really nice and do a
>   > great job of quickly making me aware of what's going on.  
> 
> Thanks!
> 
>   > I would love it if asynchronous vc commands used a similar indicator
>   > to show their status.
> 
> They should currently show "[waiting...]".  If you can think of
> something better, please fix the message.  If the asynchronous commands
> don't show this message, then it's a bug that needs fixing, so please
> say when you find such a case.
> 
> 

I noticed that doing vc-status in a CVS controlled directory displays 
[waiting...] forever, even after the error message that says CVS doesn't 
support vc-status.

	Jan D.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Jan Djärv <jan.h.d@swipnet.se> writes:

  > Dan Nicolaescu skrev:
  > > Miles Bader <miles.bader@necel.com> writes:
  > >
  > >   > I think the highlighted mode-line status indicators used by
  > >   > e.g. compile-mode (running or finished etc) now are really nice and do a
  > >   > great job of quickly making me aware of what's going on.  
  > >
  > > Thanks!
  > >
  > >   > I would love it if asynchronous vc commands used a similar indicator
  > >   > to show their status.
  > >
  > > They should currently show "[waiting...]".  If you can think of
  > > something better, please fix the message.  If the asynchronous commands
  > > don't show this message, then it's a bug that needs fixing, so please
  > > say when you find such a case.
  > >
  > >
  > 
  > I noticed that doing vc-status in a CVS controlled directory displays
  > [waiting...] forever, even after the error message that says CVS
  > doesn't support vc-status.

Thanks, I'll take a look.

Re: vc-set-mode-line-busy-indicator

[Stefan Monnier]

> I noticed that doing vc-status in a CVS controlled directory displays
> [waiting...] forever, even after the error message that says CVS doesn't
> support vc-status.

That's right.  In what way is this a bug?
Maybe we could make the message more clear in this case, something like

  [waiting for someone to implement the support for CVS...]


        Stefan ;-)

Re: vc-set-mode-line-busy-indicator

[Jan Djärv]

Stefan Monnier skrev:
>> I noticed that doing vc-status in a CVS controlled directory displays
>> [waiting...] forever, even after the error message that says CVS doesn't
>> support vc-status.
> 
> That's right.  In what way is this a bug?
> Maybe we could make the message more clear in this case, something like
> 
>   [waiting for someone to implement the support for CVS...]
> 
> 
>         Stefan ;-)
> 


[waiting...]
[loading ai.elc]
[implementing...]
[continuing...]

or something?

	Jan D.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    I dunno, maybe it's just a personal foible, but emacs the "diff" command
    used to add this sort of terminator line (haven't noticed recently) sort
    of terminator string, and I always found it very annoying -- it probably
    doesn't cause anything to _break_ but it was like an ugly little wart,
    and I always ended up going to extra effort to delete it when saving the
    output.

I think you're making a mountain out of a molehill.  If there is no
other reason for its removal, we should put it back in.

    I think the highlighted mode-line status indicators used by
    e.g. compile-mode (running or finished etc) now are really nice and do a
    great job of quickly making me aware of what's going on.

Those canbe useful, but inserting text at the end is also useful.  For
instance, M-x compile does both.  When it finishes, it inserts text
saying "Compilation finished ...".

    It forces you to do yet another operation (delete the extra output)
    every time when copying from the diff buffer to and email message, or
    some other place.

You don't need to delete that extra output.  It doesn't cause any harm
if you copy it into an email message.

Diffs between %s and %s end here (was: vc-set-mode-line-busy-indicator)

[Reiner Steib]

On Sat, Mar 08 2008, Richard Stallman wrote:

>     I dunno, maybe it's just a personal foible, but emacs the "diff" command
>     used to add this sort of terminator line (haven't noticed recently) sort
>     of terminator string, and I always found it very annoying -- it probably
>     doesn't cause anything to _break_ but it was like an ugly little wart,
>     and I always ended up going to extra effort to delete it when saving the
>     output.
>
> I think you're making a mountain out of a molehill.  

Before "Diffs between %s and %s end here" was removed, several people
reported to be distracted or annoyed by it.  See the thread on
emacs-devel in January[1].  Nobody spoke up in favor of it when Glenn
announced to "remove it in a week or so if no-one objects."

> If there is no other reason for its removal, we should put it back
> in.

Please don't (at least provide an option to turn it off).

When I produce a diff before committing a change, I often want to make
sure the no unintended/other changes crept in.  I expect to see
exactly the diffs, not some disturbing additional lines.  Even after
seeing it several weeks (but not using diff very frequently), I still
found it distracting.[1]

>     I think the highlighted mode-line status indicators used by
>     e.g. compile-mode (running or finished etc) now are really nice and do a
>     great job of quickly making me aware of what's going on.
>
> Those canbe useful, but inserting text at the end is also useful.  For
> instance, M-x compile does both.  When it finishes, it inserts text
> saying "Compilation finished ...".

I almost never want to save compilation output buffers (or include it
in a mail message), but I often do for diffs.  Additionally, in
compilation mode, it is more clear that this line is not part of the
output because the bold words are in green or red:

| Compilation *exited abnormally* with code ...
| Compilation *finished* at ...

>     It forces you to do yet another operation (delete the extra output)
>     every time when copying from the diff buffer to and email message, or
>     some other place.
>
> You don't need to delete that extra output.  It doesn't cause any harm
> if you copy it into an email message.

Not everyone is so familiar with the patch command to know what is
ignored and what not.  If even Emacs developers who are quite familiar
with patch, remove the extra line, I'm sure that most "ordinary" users
will do the extra work as well to be sure.

Bye, Reiner.

[1] "Diffs between %s and %s end here".  See the thread with this
    subject in January.
    <http://thread.gmane.org/gmane.emacs.devel/84272/focus=86974>
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Diffs between %s and %s end here

[David Kastrup]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Sat, Mar 08 2008, Richard Stallman wrote:
>
>>     I dunno, maybe it's just a personal foible, but emacs the "diff" command
>>     used to add this sort of terminator line (haven't noticed recently) sort
>>     of terminator string, and I always found it very annoying -- it probably
>>     doesn't cause anything to _break_ but it was like an ugly little wart,
>>     and I always ended up going to extra effort to delete it when saving the
>>     output.
>>
>> I think you're making a mountain out of a molehill.  
>
> Before "Diffs between %s and %s end here" was removed, several people
> reported to be distracted or annoyed by it.  See the thread on
> emacs-devel in January[1].  Nobody spoke up in favor of it when Glenn
> announced to "remove it in a week or so if no-one objects."
>
>> If there is no other reason for its removal, we should put it back
>> in.
>
> Please don't (at least provide an option to turn it off).
>
> When I produce a diff before committing a change, I often want to make
> sure the no unintended/other changes crept in.  I expect to see
> exactly the diffs, not some disturbing additional lines.  Even after
> seeing it several weeks (but not using diff very frequently), I still
> found it distracting.[1]

Maybe we should put informational messages like that as an (evaporating)
afterstring overlay on the final LF character?  That way they would not
be copied anywhere.

>> Those canbe useful, but inserting text at the end is also useful.  For
>> instance, M-x compile does both.  When it finishes, it inserts text
>> saying "Compilation finished ...".
>
> I almost never want to save compilation output buffers (or include it
> in a mail message), but I often do for diffs.  Additionally, in
> compilation mode, it is more clear that this line is not part of the
> output because the bold words are in green or red:

Same could be done here.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Diffs between %s and %s end here

[Dan Nicolaescu]

David Kastrup <dak@gnu.org> writes:

  > Reiner Steib <reinersteib+gmane@imap.cc> writes:
  > 
  > > On Sat, Mar 08 2008, Richard Stallman wrote:
  > >
  > >>     I dunno, maybe it's just a personal foible, but emacs the "diff" command
  > >>     used to add this sort of terminator line (haven't noticed recently) sort
  > >>     of terminator string, and I always found it very annoying -- it probably
  > >>     doesn't cause anything to _break_ but it was like an ugly little wart,
  > >>     and I always ended up going to extra effort to delete it when saving the
  > >>     output.
  > >>
  > >> I think you're making a mountain out of a molehill.  
  > >
  > > Before "Diffs between %s and %s end here" was removed, several people
  > > reported to be distracted or annoyed by it.  See the thread on
  > > emacs-devel in January[1].  Nobody spoke up in favor of it when Glenn
  > > announced to "remove it in a week or so if no-one objects."
  > >
  > >> If there is no other reason for its removal, we should put it back
  > >> in.
  > >
  > > Please don't (at least provide an option to turn it off).
  > >
  > > When I produce a diff before committing a change, I often want to make
  > > sure the no unintended/other changes crept in.  I expect to see
  > > exactly the diffs, not some disturbing additional lines.  Even after
  > > seeing it several weeks (but not using diff very frequently), I still
  > > found it distracting.[1]
  > 
  > Maybe we should put informational messages like that as an (evaporating)
  > afterstring overlay on the final LF character?  That way they would not
  > be copied anywhere.

That sounds good, as long as M-w and insert-buffer don't copy that
string.
But the main question remains: is it worth doing it? We already have the
mode-line notification, does adding another type provide a significant
benefit for the user?

Re: Diffs between %s and %s end here

[David Kastrup]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> David Kastrup <dak@gnu.org> writes:
>
>   > Maybe we should put informational messages like that as an
>   > (evaporating) afterstring overlay on the final LF character?  That
>   > way they would not be copied anywhere.
>
> That sounds good, as long as M-w and insert-buffer don't copy that
> string.

Huh?  Overlays are not a part of the buffer text.

> But the main question remains: is it worth doing it? We already have
> the mode-line notification, does adding another type provide a
> significant benefit for the user?

Its relevance is after the compilation text, so it is a natural place to
look for it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Diffs between %s and %s end here

[Juri Linkov]

>>> If there is no other reason for its removal, we should put it back in.
>>
>> Please don't (at least provide an option to turn it off).
>>
>> When I produce a diff before committing a change, I often want to make
>> sure the no unintended/other changes crept in.  I expect to see
>> exactly the diffs, not some disturbing additional lines.  Even after
>> seeing it several weeks (but not using diff very frequently), I still
>> found it distracting.[1]
>
> Maybe we should put informational messages like that as an (evaporating)
> afterstring overlay on the final LF character?  That way they would not
> be copied anywhere.

I think this is a good idea: this footer will be still displayed
but not copied or saved.

But please don't use such overlays for compilation buffers - they are not
like diff output.  After saving and later visiting the old compilation
output it is useful to see the overall status of the compilation at the
end of the saved output file.  Also it is useful to see how long the full
compilation took time (by subtracting times at the beginning and the end
of compilation) to make estimation how long the next compilation will
take time.  However, nothing like this is necessary for diff output
which I think should have no additional text appended in the saved file.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Diffs between %s and %s end here

[Glenn Morris]

Juri Linkov wrote:

>> Maybe we should put informational messages like that as an (evaporating)
>> afterstring overlay on the final LF character?  That way they would not
>> be copied anywhere.
>
> I think this is a good idea: this footer will be still displayed
> but not copied or saved.

I don't. I object to the visual appearance of this footer in any form.

Re: Diffs between %s and %s end here

[Juri Linkov]

>>> Maybe we should put informational messages like that as an (evaporating)
>>> afterstring overlay on the final LF character?  That way they would not
>>> be copied anywhere.
>>
>> I think this is a good idea: this footer will be still displayed
>> but not copied or saved.
>
> I don't. I object to the visual appearance of this footer in any form.

My comment was for the case when displaying this footer is unavoidable
(though I don't think it is), and provided it will be optional, I will
immediately turn it off.

Since it seems the discussion currently reached consensus that
additional text is necessary only for visual indication, I have
a question: isn't visual indication for the current status
in the mode line is sufficient with the new face `mode-line-emphasis'?
Maybe we should install a change that uses this new face to display the
status, and wait for more feedback?

Then what about *cvs-diff* and *Diff*?  Should they use the same technique?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Diffs between %s and %s end here

[Glenn Morris]

Juri Linkov wrote:

> Since it seems the discussion currently reached consensus that
> additional text is necessary only for visual indication, I have
> a question: isn't visual indication for the current status
> in the mode line is sufficient with the new face `mode-line-emphasis'?

I (have always) agree(d) with this, but I'm not sure if rms does.

Re: Diffs between %s and %s end here

[David Kastrup]

Glenn Morris <rgm@gnu.org> writes:

> Juri Linkov wrote:
>
>> Since it seems the discussion currently reached consensus that
>> additional text is necessary only for visual indication, I have
>> a question: isn't visual indication for the current status
>> in the mode line is sufficient with the new face `mode-line-emphasis'?
>
> I (have always) agree(d) with this, but I'm not sure if rms does.

For me, the bottom of the compilation output seems like a natural place
to look for that kind of info.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Diffs between %s and %s end here

[Dan Nicolaescu]

David Kastrup <dak@gnu.org> writes:

  > Glenn Morris <rgm@gnu.org> writes:
  > 
  > > Juri Linkov wrote:
  > >
  > >> Since it seems the discussion currently reached consensus that
  > >> additional text is necessary only for visual indication, I have
  > >> a question: isn't visual indication for the current status
  > >> in the mode line is sufficient with the new face `mode-line-emphasis'?
  > >
  > > I (have always) agree(d) with this, but I'm not sure if rms does.
  > 
  > For me, the bottom of the compilation output seems like a natural place
  > to look for that kind of info.

It has never been there, adding it (like adding anything else to emacs)
should require a very strong argument, given there are so many counter
arguments.

Re: Diffs between %s and %s end here

[David Kastrup]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> David Kastrup <dak@gnu.org> writes:
>
>   > Glenn Morris <rgm@gnu.org> writes:
>   > 
>   > > Juri Linkov wrote:
>   > >
>   > >> Since it seems the discussion currently reached consensus that
>   > >> additional text is necessary only for visual indication, I have
>   > >> a question: isn't visual indication for the current status
>   > >> in the mode line is sufficient with the new face `mode-line-emphasis'?
>   > >
>   > > I (have always) agree(d) with this, but I'm not sure if rms does.
>   > 
>   > For me, the bottom of the compilation output seems like a natural place
>   > to look for that kind of info.
>
> It has never been there, adding it (like adding anything else to emacs)
> should require a very strong argument, given there are so many counter
> arguments.

Huh?


[-- Attachment #2: Type: text/plain, Size: 158 bytes --]

-*- mode: compilation; default-directory: "/home/tmp/akt/" -*-
Compilation started at Sun Mar  9 11:09:45

echo


Compilation finished at Sun Mar  9 11:09:46

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Diffs between %s and %s end here

[Dan Nicolaescu]

David Kastrup <dak@gnu.org> writes:

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > > David Kastrup <dak@gnu.org> writes:
  > >
  > >   > Glenn Morris <rgm@gnu.org> writes:
  > >   > 
  > >   > > Juri Linkov wrote:
  > >   > >
  > >   > >> Since it seems the discussion currently reached consensus that
  > >   > >> additional text is necessary only for visual indication, I have
  > >   > >> a question: isn't visual indication for the current status
  > >   > >> in the mode line is sufficient with the new face `mode-line-emphasis'?
  > >   > >
  > >   > > I (have always) agree(d) with this, but I'm not sure if rms does.
  > >   > 
  > >   > For me, the bottom of the compilation output seems like a natural place
  > >   > to look for that kind of info.
  > >
  > > It has never been there, adding it (like adding anything else to emacs)
  > > should require a very strong argument, given there are so many counter
  > > arguments.
  > 
  > Huh?

I was talking about diff.

Re: Diffs between %s and %s end here

[Richard Stallman]

      > For me, the bottom of the compilation output seems like a natural place
      > to look for that kind of info.

    It has never been there, adding it (like adding anything else to emacs)
    should require a very strong argument, given there are so many counter
    arguments.

In another message I've explained why it is useful.
In addition, it is desirable for consistency with M-x diff.

Re: Diffs between %s and %s end here

[Richard Stallman]

    Since it seems the discussion currently reached consensus that
    additional text is necessary only for visual indication, I have
    a question: isn't visual indication for the current status
    in the mode line is sufficient with the new face `mode-line-emphasis'?

I don't think so.  When the process terminates, it is nice to have
confirmation that it completed normally.

Re: Diffs between %s and %s end here

[Juri Linkov]

>     Since it seems the discussion currently reached consensus that
>     additional text is necessary only for visual indication, I have
>     a question: isn't visual indication for the current status
>     in the mode line is sufficient with the new face `mode-line-emphasis'?
>
> I don't think so.  When the process terminates, it is nice to have
> confirmation that it completed normally.

Then we could use the same non-intrusive solution already implemented for
compile: when compilation is successful it displays the successful status
in the mode line of the compilation buffer in green.  When compilation
fails, it displays the error code in the mode line in red.

So no additional text is necessary to append to the end of the output that
many find too annoying.  This modeline indication should be enough for
confirmation the process completed normally.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Diffs between %s and %s end here

[Richard Stallman]

    Then we could use the same non-intrusive solution already implemented for
    compile: when compilation is successful it displays the successful status
    in the mode line of the compilation buffer in green.  When compilation
    fails, it displays the error code in the mode line in red.

Using faces that way seems like a good idea.  Still, the arguments
offered against inserting a "finished" message, like M-x compile
and M-x diff, seem to make much ado about very little.

Re: Diffs between %s and %s end here

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     Then we could use the same non-intrusive solution already implemented for
>     compile: when compilation is successful it displays the successful status
>     in the mode line of the compilation buffer in green.  When compilation
>     fails, it displays the error code in the mode line in red.
>
> Using faces that way seems like a good idea.  Still, the arguments
> offered against inserting a "finished" message, like M-x compile
> and M-x diff, seem to make much ado about very little.

As long as there is reasonable agreement that a "very little" change is
an improvement, I don't see anything wrong in following through.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Diffs between %s and %s end here

[Glenn Morris]

Richard Stallman wrote:

>     Since it seems the discussion currently reached consensus that
>     additional text is necessary only for visual indication, I have
>     a question: isn't visual indication for the current status
>     in the mode line is sufficient with the new face `mode-line-emphasis'?
>
> I don't think so.  When the process terminates, it is nice to have
> confirmation that it completed normally.

It's very frustrating that a discussion about creating a single new
face has got sidetracked into this issue. We have already had this
discussion. No-one agreed with you. Please read the references Reiner
supplied.

Meanwhile, vc-set-mode-line-busy-indicator continue to be totally
invisible for me.

Re: Diffs between %s and %s end here

[Stefan Monnier]

>> Since it seems the discussion currently reached consensus that
>> additional text is necessary only for visual indication, I have
>> a question: isn't visual indication for the current status
>> in the mode line is sufficient with the new face `mode-line-emphasis'?
>> 
>> I don't think so.  When the process terminates, it is nice to have
>> confirmation that it completed normally.

> It's very frustrating that a discussion about creating a single new
> face has got sidetracked into this issue. We have already had this
> discussion. No-one agreed with you. Please read the references Reiner
> supplied.

> Meanwhile, vc-set-mode-line-busy-indicator continue to be totally
> invisible for me.

Please create a mode-line-emphasis face and use it for
vc-set-mode-line-busy-indicator.


        Stefan

Re: Diffs between %s and %s end here

[Richard Stallman]

    It's very frustrating that a discussion about creating a single new
    face has got sidetracked into this issue. We have already had this
    discussion. No-one agreed with you. Please read the references Reiner
    supplied.

Due to the rather nasty tone, I do not feel inclined to carry out your
demand.  Especially when it involves something as inconvenient as
looking at a web site.

Re: Diffs between %s and %s end here

[Glenn Morris]

Richard Stallman wrote:

> Due to the rather nasty tone, I do not feel inclined to carry out your
> demand.

You've accused me of making demands before. I'm not at all put out,
but rather than letting it slide this time, let's analyse my mail:

  It's very frustrating

A statement of how I feel about something. In hindsight, the "very"
was overkill, but I guess I'm entitled to feel how I like.

  that a discussion about creating a single new face has got
  sidetracked into this issue.

A statement that I think the discussion has been sidetracked.

  We have already had this discussion.

A statement of fact.

  No-one agreed with you.

A statement of fact.

  Please read the references Reiner supplied.

A request framed in a normal level of politeness. I don't know how you
want me to ask you things if you think this is a "demand".

>  Especially when it involves something as inconvenient as looking at
> a web site.

Reiner also supplied the name of the thread and the month it took
place in. I thought this was how you normally found old mailing list
discussions.

Re: Diffs between %s and %s end here

[Richard Stallman]

    > Due to the rather nasty tone, I do not feel inclined to carry out your
    > demand.

    You've accused me of making demands before. I'm not at all put out,
    but rather than letting it slide this time, let's analyse my mail:

Tone isn't found in the questions that you analyzed.  For instance, a
demand can be made in the words of a polite request.

Perhaps you didn't mean it as nasty.  Perhaps you didn't mean to say
"Go read that link (and shut up)."  But that's what it sounds like to
me.

Re: Diffs between %s and %s end here (was: vc-set-mode-line-busy-indicator)

[Richard Stallman]

    When I produce a diff before committing a change, I often want to make
    sure the no unintended/other changes crept in.  I expect to see
    exactly the diffs, not some disturbing additional lines.

How do you check that they are exactly the same diffs?
Do you use M-x compare-windows?  When that comes to the end
of the diffs in both buffers, the fact that one has a note
saying "end of diffs" won't make it harder for you to tell
that the two are identical.

    I almost never want to save compilation output buffers (or include it
    in a mail message), but I often do for diffs.

I don't think that end-of-diffs message will do any harm when you save
or mail the diffs.  In an email there will usually be other text both
above and below the diffs, in any case.  Adding one line of more text
below the diffs won't change the situation much.

						   Additionally, in
    compilation mode, it is more clear that this line is not part of the
    output because the bold words are in green or red:

We could put colors into the notice in the diff buffer
just as we do in compilation mode.

Re: Diffs between %s and %s end here

[Reiner Steib]

On Sun, Mar 09 2008, Richard Stallman wrote:

>     When I produce a diff before committing a change, I often want to make
>     sure the no unintended/other changes crept in.  I expect to see
>     exactly the diffs, not some disturbing additional lines.
>
> How do you check that they are exactly the same diffs?
> Do you use M-x compare-windows?  

No.  Usually I just look at it.  E.g. when someone sends a mail with a
patch for Gnus, I apply it from the mail buffer.  Then I do `C-x v ='
(or `=' in PCL-CVS) and check if everything is as intended (e.g. that
there are no other unrelated uncommitted changes).

> When that comes to the end of the diffs in both buffers, the fact
> that one has a note saying "end of diffs" won't make it harder for
> you to tell that the two are identical.
[...]
> I don't think that end-of-diffs message will do any harm when you save
> or mail the diffs.  

I can only repeat that even after seeing this note several time, I was
still distracted.  We already had such a end-of-diff note in CVS (see
the subject: "Diffs between %s and %s end here") for some weeks and
_nobody_ argued to keep it, but several developers were and still are
strongly against it.

> 						   Additionally, in
>     compilation mode, it is more clear that this line is not part of the
>     output because the bold words are in green or red:
>
> We could put colors into the notice in the diff buffer just as we do
> in compilation mode.

I agree with Juri (and others)...;
in <87k5kb7pm9.fsf@jurta.org>, Juri wrote:
| Then we could use the same non-intrusive solution already implemented for
| compile: when compilation is successful it displays the successful status
| in the mode line of the compilation buffer in green.  When compilation
| fails, it displays the error code in the mode line in red.
| 
| So no additional text is necessary to append to the end of the output that
| many find too annoying.  This modeline indication should be enough for
| confirmation the process completed normally.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >     I dunno, maybe it's just a personal foible, but emacs the "diff" command
  >     used to add this sort of terminator line (haven't noticed recently) sort
  >     of terminator string, and I always found it very annoying -- it probably
  >     doesn't cause anything to _break_ but it was like an ugly little wart,
  >     and I always ended up going to extra effort to delete it when saving the
  >     output.
  > 
  > I think you're making a mountain out of a molehill.  If there is no
  > other reason for its removal, we should put it back in.

I think you misunderstand, this has not been done in any released
version of emacs, it was just a failed experiment in CVS HEAD.

  >     I think the highlighted mode-line status indicators used by
  >     e.g. compile-mode (running or finished etc) now are really nice and do a
  >     great job of quickly making me aware of what's going on.
  > 
  > Those canbe useful, but inserting text at the end is also useful.  For
  > instance, M-x compile does both.  When it finishes, it inserts text
  > saying "Compilation finished ...".

Compilation buffers are fundamentally different: people don't email them, read
the with a lot of attention, and don't pass them to other tools, so this
argument does not apply.


  >     It forces you to do yet another operation (delete the extra output)
  >     every time when copying from the diff buffer to and email message, or
  >     some other place.
  > 
  > You don't need to delete that extra output.  It doesn't cause any harm
  > if you copy it into an email message.

I does, I've had a bad personal experience with that: after adding
patches at the end of mail messages with  using M-x insert-buffer RET *vc-diff* RET 
I got a comment: "Please fix your mailer not to insert crap at the end of messages"
There's no reason to subject emacs users to such abuse for absolutely
zero benefit.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

      > I think you're making a mountain out of a molehill.  If there is no
      > other reason for its removal, we should put it back in.

    I think you misunderstand, this has not been done in any released
    version of emacs, it was just a failed experiment in CVS HEAD.

You have lost me there.  I don't know what that sentence means.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >       > I think you're making a mountain out of a molehill.  If there is no
  >       > other reason for its removal, we should put it back in.
  > 
  >     I think you misunderstand, this has not been done in any released
  >     version of emacs, it was just a failed experiment in CVS HEAD.
  > 
  > You have lost me there.  I don't know what that sentence means.

It means that such a feature has never been available in an emacs
release, so it did not exist for the vast majority of emacs users.

To add such a feature now, it has to stand up to the usual scrutiny that
new features usually do.

Up until now it has completely failed this scrutiny. Except for you,
everyone on this list has hated it, and given good reasons for that.

You said in another message that you explained why this would be good. 

But many people in many different messages have showed that it will
definitely be bad, even when taking into account your arguments.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    I does, I've had a bad personal experience with that: after adding
    patches at the end of mail messages with  using M-x insert-buffer RET *vc-diff* RET 
    I got a comment: "Please fix your mailer not to insert crap at the end of messages"

It sounds like you were dealing with someone rude.

    There's no reason to subject emacs users to such abuse for absolutely
    zero benefit.

I've explained elsewhere the benefit.

Re: vc-set-mode-line-busy-indicator

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >     Please don't, there temporarily was some for a few months, and it was
  >     discussed at length on the list and voted out.  It is annoying when you
  >     have to copy the buffer that there's extra text that is not part of the
  >     diff that you have to deal with.
  > 
  > Suppose it says "End of diff output".  

That's approximately what it used to say, and it was very much hated, and voted out.

  > Why is it a problem if you copy that along with the diff?  If you
  > copy that, what harm does it do?

It forces you to do yet another operation (delete the extra output)
every time when copying from the diff buffer to and email message, or
some other place.  We do have a superior, very intuitive solution
(i.e. displaying the running status in the mode-line), that does not
require extra work, so it is not a good idea to go back to something
that does not provide any advantage, but requires extra work in the case
of copying from the diff buffer (which nowadays is quite a frequent
operations, many many patches get emailed between developers).

  > It won't bother `patch'.

It will bother the reader though.

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Richard Stallman wrote:

>     I suggest a new `mode-line-emphasis' face. The existing
>     `mode-line-highlight' doesn't look right because it is designed for
>     buttons. I made up some defaults; I'm not sure about them.
>
> Does this proposal mean having two different faces for two different
> kinds of highlighting in the mode line?

Yes. mode-line-highlight (as now) for things you might click, which are
highlighted as buttons on mouse-over. mode-line-emphasis for
"non-interactive" things you don't click, which are always highlighted.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    Yes. mode-line-highlight (as now) for things you might click, which are
    highlighted as buttons on mouse-over. mode-line-emphasis for
    "non-interactive" things you don't click, which are always highlighted.

1. Do we really need both?  (I am worried about face bloat.)

2. If we need both, the name of the one for buttons should be
`mode-line-button' and the one for emphasis should be
`mode-line-highlight'.

3. Can we make the one for buttons by means of `button'
instead of having a separate face only for mode line buttons?

Re: vc-set-mode-line-busy-indicator

[Glenn Morris]

Richard Stallman wrote:

>     Yes. mode-line-highlight (as now) for things you might click, which are
>     highlighted as buttons on mouse-over. mode-line-emphasis for
>     "non-interactive" things you don't click, which are always highlighted.
>
> 1. Do we really need both?  (I am worried about face bloat.)

No. vc-set-mode-line-busy-indicator can not apply a 'face property for
all I care. (I knew this worry about "face bloat" would come up. I
don't understand it.)

> 2. If we need both, the name of the one for buttons should be
> `mode-line-button' and the one for emphasis should be
> `mode-line-highlight'.

This would be an incompatible change to the use of the existing
mode-line-highlight face.

> 3. Can we make the one for buttons by means of `button'
> instead of having a separate face only for mode line buttons?

I'll say, once again, that the mode-line has a different colour scheme
to the buffer. Trying to re-use buffer faces in the mode-line can look
very poor, as at present with vc-set-mode-line-busy-indicator.


I've said all I have to say on this. I've said why the current choice
is bad, and I've offered two alternatives (using no 'face at all, or a
new one). I'll implement either. But please let's do something,
because the current behaviour is broken.

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    > 2. If we need both, the name of the one for buttons should be
    > `mode-line-button' and the one for emphasis should be
    > `mode-line-highlight'.

    This would be an incompatible change to the use of the existing
    mode-line-highlight face.

Yes, but it is better to have a small incompatible change in a minor
feature than have it remain badly named.

Re: vc-set-mode-line-busy-indicator

[Juri Linkov]

>     Yes. mode-line-highlight (as now) for things you might click, which are
>     highlighted as buttons on mouse-over. mode-line-emphasis for
>     "non-interactive" things you don't click, which are always highlighted.
>
> 1. Do we really need both?  (I am worried about face bloat.)
>
> 2. If we need both, the name of the one for buttons should be
> `mode-line-button' and the one for emphasis should be
> `mode-line-highlight'.

`mode-line-highlight' servers the same purpose as its counterpart
`highlight' that historically is used to temporarily highlight text
during mouse-overs.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    > 2. If we need both, the name of the one for buttons should be
    > `mode-line-button' and the one for emphasis should be
    > `mode-line-highlight'.

    `mode-line-highlight' servers the same purpose as its counterpart
    `highlight' that historically is used to temporarily highlight text
    during mouse-overs.

They do not seem similar to me.  `highlight' is used for highlighting
text that is always present, and could be used on any text.  My
understanding is that `mode-line-highlight' is proposed for use on a
particular notice that will never appear without that.

    Since now Richard argued even for using no mode line indication at all,
    bold doesn't seem too bad.  It is not strong and still helps to notice
    the status of the vc command.

I am not arguing strongly against it.  Sometimes it is worth making
a new face.  I just want people not to do this hastily.

Re: vc-set-mode-line-busy-indicator

[Juri Linkov]

>     > 2. If we need both, the name of the one for buttons should be
>     > `mode-line-button' and the one for emphasis should be
>     > `mode-line-highlight'.
>
>     `mode-line-highlight' servers the same purpose as its counterpart
>     `highlight' that historically is used to temporarily highlight text
>     during mouse-overs.
>
> They do not seem similar to me.  `highlight' is used for highlighting
> text that is always present, and could be used on any text.

`mode-line-highlight' is currently used for the same purposes on the
mode line - for highlighting text that is always present on the mode line.

> My understanding is that `mode-line-highlight' is proposed for use on
> a particular notice that will never appear without that.

It seems no one proposes using `mode-line-highlight' on a particular
notice on the mode line.  On the contrary, a new face was proposed
to use for such notices that will never appear without that.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: vc-set-mode-line-busy-indicator

[Stefan Monnier]

> It seems no one proposes using `mode-line-highlight' on a particular
> notice on the mode line.  On the contrary, a new face was proposed
> to use for such notices that will never appear without that.

Indeed mode-line-emphasis sounds good to me,


        Stefan

Re: vc-set-mode-line-busy-indicator

[Richard Stallman]

    > They do not seem similar to me.  `highlight' is used for highlighting
    > text that is always present, and could be used on any text.

    `mode-line-highlight' is currently used for the same purposes on the
    mode line - for highlighting text that is always present on the mode line.

After checking the code, I think you are right.  Someone else compared
`mode-line-highlight' to buttons, so I based what I said on that, but
now I think that isn't really so.  This text is only partly like a button.

Re: vc-set-mode-line-busy-indicator

[Stefan Monnier]

>> > >  > ! 	(propertize " [waiting...]"
>> > >  > ! 		    'face 'font-lock-variable-name-face

Why would `font-lock-variable-name-face' be a good face for it?
It seems completely arbitrary.


        Stefan

Re: vc-set-mode-line-busy-indicator

[Nick Roberts]

 > >> > >  > ! 	(propertize " [waiting...]"
 > >> > >  > ! 		    'face 'font-lock-variable-name-face
 > 
 > Why would `font-lock-variable-name-face' be a good face for it?
 > It seems completely arbitrary.

Suggest a better alternative then (Glenn already has).

-- 
Nick                                           http://www.inet.net.nz/~nickrob

diff-mode misinterprets empty lines.

[Richard Stallman]

This is important to fix.  Would someone please fix it in Emacs 22,
then ack?


X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY 
	autolearn=failed version=3.1.0
To: bug-gnu-emacs@gnu.org
From: Sergei Organov <osv@javad.com>
Date: Tue, 27 Nov 2007 21:04:17 +0300
Message-ID: <8763zno99a.fsf@osv.gnss.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: diff-mode misinterprets empty lines.

Hello,

In GNU Emacs 22.1.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20).

GNU diff since version 2.8.7 outputs empty lines in the unified diff
format for empty context lines that don't change (instead of single
space followed by newline). See, e.g., the thread starting here:

<http://lists.gnu.org/archive/html/bug-gnu-utils/2006-09/msg00005.html>

Emacs diff-mode interprets empty lines as patch hunk separators, in
particular leading to wrong calculation of the length put into hunk
header. Another symptom is "C-c C-n" (diff-restrict-view) wrongly
restricts hunks with empty line(s).

Here is a patch hunk to test on:

@@ -1,7 +1,7 @@
 l1

 l3
-	  abc
+	  cba
 l5
 l6
 l7


-- 
Sergei Organov.

Re: diff-mode misinterprets empty lines.

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

> This is important to fix.  Would someone please fix it in Emacs 22,
> then ack?

[...]

> GNU diff since version 2.8.7 outputs empty lines in the unified diff
> format for empty context lines that don't change (instead of single
> space followed by newline). See, e.g., the thread starting here:
>
> <http://lists.gnu.org/archive/html/bug-gnu-utils/2006-09/msg00005.html>

I have to agree with those thinking it is a terrible idea to prune
trailing spaces that are part of the _diff_ format.

When people don't want trailing whitespace warnings to trigger in their
editor, they should obviously amend diff-mode for that.  But changing
the diff format instead is insane.  And that patch happens to have
tolerance code intended to deal with utterly broken mailers is no excuse
for that.

There are lots of other tools working with diffs and applying patches.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: diff-mode misinterprets empty lines.

[Stefan Monnier]

>> GNU diff since version 2.8.7 outputs empty lines in the unified diff
>> format for empty context lines that don't change (instead of single
>> space followed by newline). See, e.g., the thread starting here:
>> 
>> <http://lists.gnu.org/archive/html/bug-gnu-utils/2006-09/msg00005.html>

> I have to agree with those thinking it is a terrible idea to prune
> trailing spaces that are part of the _diff_ format.

Agreed as well, this is a terrible idea.  Making tools somewhat robust
in the face of lost trailing whitespace is good (diff-mode does that),
but actively removing them is bad.

What's the benefit, really?
The original post said "This doesn't really matter..." and I agree with
this part.  So breaking every/most program that parses diffs just
because "when the only trailing blanks in a diff are those in context headers,
it's a little annoying" seems gratuitous.

Could you please revert this change?


        Stefan "Author of Emacs's diff-mode"

Re: diff-mode misinterprets empty lines.

[Paul Eggert]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Could you please revert this change?

I did so, by installing the following patch into diffutils.  The old
behavior (with trailing blanks) is now the default, and the new
behavior (without them) is enabled only if the new
--suppress-blank-empty option is given.

However, the next POSIX version specifies a "diff -u" format in which
those trailing blanks are optional, and apparently some "diff"
implementations in the wild omit them even though no official or test
version of GNU diff ever did.  So, as a practical matter, programs
that parse "diff" output should not assume the trailing blanks are
present, regardless of what GNU diff does.

2007-12-04  Paul Eggert  <eggert@cs.ucla.edu>

	* NEWS: New diff option --suppress-blank-empty (no longer -u default).
	* doc/diff.texi (Detailed Unified): Trailing spaces are no longer
	omitted.
	(Trailing Blanks): New section.
	(diff Options) Mention new option.
	* src/diff.h (suppress_blank_entry): New decl.
	* src/context.c (pr_unidiff_hunk): Support --suppress-blank-empty.
	* src/util.c (print_1_line): Likewise.
	* src/diff.c (longopts, main, option_help_msgid): Likewise.
	(SUPPRESS_BLANK_EMPTY_OPTION): New constant.

Index: NEWS
===================================================================
RCS file: /cvsroot/diffutils/diffutils/NEWS,v
retrieving revision 1.27
diff -u -p -r1.27 NEWS
--- NEWS	19 Jul 2007 17:45:26 -0000	1.27
+++ NEWS	5 Dec 2007 07:24:10 -0000
@@ -1,7 +1,6 @@
 User-visible changes since 2.8.7 (in "version" 2.8.7-cvs):

-* When reporting an unchanged empty line, diff -u now outputs an
-  empty line instead of a line consisting of a single space character.
+* New diff option --suppress-blank-empty.

 * Bring back support for `diff -NUM', where NUM is a number,
   even when conforming to POSIX 1003.1-2001.  This change reverts to
Index: doc/diff.texi
===================================================================
RCS file: /cvsroot/diffutils/diffutils/doc/diff.texi,v
retrieving revision 1.33
diff -u -p -r1.33 diff.texi
--- doc/diff.texi	15 Aug 2007 19:33:36 -0000	1.33
+++ doc/diff.texi	5 Dec 2007 07:24:11 -0000
@@ -842,8 +842,7 @@ line numbers look like @samp{@var{start}
 its end line number appears.  An empty hunk is considered to end at
 the line that precedes the hunk.

-Lines common to both files begin with a space character, except that
-the space is omitted if its line is empty.  Lines
+The lines common to both files begin with a space character.  The lines
 that actually differ between the two files have one of the following
 indicator characters in the left print column:

@@ -1854,8 +1853,9 @@ specified patterns.
 These adjustments can be applied to any output format.

 @menu
-* Tabs::       Preserving the alignment of tab stops.
-* Pagination:: Page numbering and time-stamping @command{diff} output.
+* Tabs::            Preserving the alignment of tab stops.
+* Trailing Blanks:: Suppressing blanks before empty output lines.
+* Pagination::      Page numbering and time-stamping @command{diff} output.
 @end menu

 @node Tabs
@@ -1890,6 +1890,29 @@ output format, which does not have a spa
 type indicator character.  Select this method with the @option{-T} or
 @option{--initial-tab} option.

+@node Trailing Blanks
+@section Omitting trailing blanks
+@cindex trailing blanks
+When outputting lines in normal or context format, or outputting an
+unchanged line in unified format, @command{diff} normally outputs a
+blank just before each line.  If the line is empty, the output of
+@command{diff} therefore contains trailing blanks even though the
+input does not contain them.  For example, when outputting an
+unchanged empty line in context format, @command{diff} normally
+outputs a line with two leading spaces.
+
+Some text editors and email agents routinely delete trailing blanks,
+so it can be a problem to deal with diff output files that contain
+them.  You can avoid this problem with the
+@option{--suppress-blank-empty} option.  It causes @command{diff} to
+omit trailing blanks at the end of output lines in normal, context,
+and unified format, unless the trailing blanks were already present in
+the input.  This changes the output format slightly, so that output
+lines are guaranteed to never end in a blank unless an input line ends
+in a blank.  This format is less likely to be munged by text editors
+or by transmission via email.  It is accepted by @acronym{GNU}
+@command{patch} as well.
+
 @node Pagination
 @section Paginating @command{diff} Output
 @cindex paginating @command{diff} output
@@ -3878,6 +3901,11 @@ normal.  @xref{Tabs}.
 Assume that tab stops are set every @var{columns} (default 8) print
 columns.  @xref{Tabs}.

+@item --suppress-blank-empty
+Suppress any blanks before newlines when printing the representation
+of an empty line, when outputting normal, context, or unified format.
+@xref{Trailing Blanks}.
+
 @item --to-file=@var{file}
 Compare each operand to @var{file}; @var{file} may be a directory.

Index: src/diff.h
===================================================================
RCS file: /cvsroot/diffutils/diffutils/src/diff.h,v
retrieving revision 1.28
diff -u -p -r1.28 diff.h
--- src/diff.h	19 Jul 2007 17:45:29 -0000	1.28
+++ src/diff.h	5 Dec 2007 07:24:11 -0000
@@ -150,6 +150,9 @@ XTERN size_t tabsize;
    without changing the characters in it (-T).  */
 XTERN bool initial_tab;

+/* Do not output an initial space or tab before the text of an empty line.  */
+XTERN bool suppress_blank_empty;
+
 /* Remove trailing carriage returns from input.  */
 XTERN bool strip_trailing_cr;

Index: src/context.c
===================================================================
RCS file: /cvsroot/diffutils/diffutils/src/context.c,v
retrieving revision 1.23
diff -u -p -r1.23 context.c
--- src/context.c	19 Jul 2007 17:45:28 -0000	1.23
+++ src/context.c	5 Dec 2007 07:24:11 -0000
@@ -350,7 +350,7 @@ pr_unidiff_hunk (struct change *hunk)
       if (!next || i < next->line0)
 	{
 	  char const *const *line = &files[0].linbuf[i++];
-	  if (**line != '\n')
+	  if (! (suppress_blank_empty && **line == '\n'))
 	    putc (initial_tab ? '\t' : ' ', out);
 	  print_1_line (NULL, line);
 	  j++;
@@ -362,10 +362,11 @@ pr_unidiff_hunk (struct change *hunk)
 	  k = next->deleted;
 	  while (k--)
 	    {
+	      char const * const *line = &files[0].linbuf[i++];
 	      putc ('-', out);
-	      if (initial_tab)
+	      if (initial_tab && ! (suppress_blank_empty && **line == '\n'))
 		putc ('\t', out);
-	      print_1_line (NULL, &files[0].linbuf[i++]);
+	      print_1_line (NULL, line);
 	    }

 	  /* Then output the inserted part. */
@@ -373,10 +374,11 @@ pr_unidiff_hunk (struct change *hunk)
 	  k = next->inserted;
 	  while (k--)
 	    {
+	      char const * const *line = &files[1].linbuf[j++];
 	      putc ('+', out);
-	      if (initial_tab)
+	      if (initial_tab && ! (suppress_blank_empty && **line == '\n'))
 		putc ('\t', out);
-	      print_1_line (NULL, &files[1].linbuf[j++]);
+	      print_1_line (NULL, line);
 	    }

 	  /* We're done with this hunk, so on to the next! */
Index: src/util.c
===================================================================
RCS file: /cvsroot/diffutils/diffutils/src/util.c,v
retrieving revision 1.37
diff -u -p -r1.37 util.c
--- src/util.c	15 Aug 2007 19:32:22 -0000	1.37
+++ src/util.c	5 Dec 2007 07:24:11 -0000
@@ -506,7 +506,8 @@ print_script (struct change *script,
 \f
 /* Print the text of a single line LINE,
    flagging it with the characters in LINE_FLAG (which say whether
-   the line is inserted, deleted, changed, etc.).  */
+   the line is inserted, deleted, changed, etc.).  LINE_FLAG must not
+   end in a blank, unless it is a single blank.  */

 void
 print_1_line (char const *line_flag, char const *const *line)
@@ -517,12 +518,25 @@ print_1_line (char const *line_flag, cha

   /* If -T was specified, use a Tab between the line-flag and the text.
      Otherwise use a Space (as Unix diff does).
-     Print neither space nor tab if line-flags are empty.  */
+     Print neither space nor tab if line-flags are empty.
+     But omit trailing blanks if requested.  */

   if (line_flag && *line_flag)
     {
-      flag_format = initial_tab ? "%s\t" : "%s ";
-      fprintf (out, flag_format, line_flag);
+      char const *flag_format_1 = flag_format = initial_tab ? "%s\t" : "%s ";
+      char const *line_flag_1 = line_flag;
+
+      if (suppress_blank_empty && **line == '\n')
+	{
+	  flag_format_1 = "%s";
+
+	  /* This hack to omit trailing blanks takes advantage of the
+	     fact that the only way that LINE_FLAG can end in a blank
+	     is when LINE_FLAG consists of a single blank.  */
+	  line_flag_1 += *line_flag_1 == ' ';
+	}
+
+      fprintf (out, flag_format_1, line_flag_1);
     }

   output_1_line (base, limit, flag_format, line_flag);
Index: src/diff.c
===================================================================
RCS file: /cvsroot/diffutils/diffutils/src/diff.c,v
retrieving revision 1.46
diff -u -p -r1.46 diff.c
--- src/diff.c	15 Aug 2007 19:32:22 -0000	1.46
+++ src/diff.c	5 Dec 2007 07:24:11 -0000
@@ -110,6 +110,7 @@ enum
   NORMAL_OPTION,
   SDIFF_MERGE_ASSIST_OPTION,
   STRIP_TRAILING_CR_OPTION,
+  SUPPRESS_BLANK_EMPTY_OPTION,
   SUPPRESS_COMMON_LINES_OPTION,
   TABSIZE_OPTION,
   TO_FILE_OPTION,
@@ -187,6 +188,7 @@ static struct option const longopts[] =
   {"speed-large-files", 0, 0, 'H'},
   {"starting-file", 1, 0, 'S'},
   {"strip-trailing-cr", 0, 0, STRIP_TRAILING_CR_OPTION},
+  {"suppress-blank-empty", 0, 0, SUPPRESS_BLANK_EMPTY_OPTION},
   {"suppress-common-lines", 0, 0, SUPPRESS_COMMON_LINES_OPTION},
   {"tabsize", 1, 0, TABSIZE_OPTION},
   {"text", 0, 0, 'a'},
@@ -565,6 +567,10 @@ main (int argc, char **argv)
 	  strip_trailing_cr = true;
 	  break;

+	case SUPPRESS_BLANK_EMPTY_OPTION:
+	  suppress_blank_empty = true;
+	  break;
+
 	case SUPPRESS_COMMON_LINES_OPTION:
 	  suppress_common_lines = true;
 	  break;
@@ -886,6 +892,7 @@ static char const * const option_help_ms
   N_("-t  --expand-tabs  Expand tabs to spaces in output."),
   N_("-T  --initial-tab  Make tabs line up by prepending a tab."),
   N_("--tabsize=NUM  Tab stops are every NUM (default 8) print columns."),
+  N_("--suppress-blank-empty  Suppress space or tab before empty output lines."),
   "",
   N_("-r  --recursive  Recursively compare any subdirectories found."),
   N_("-N  --new-file  Treat absent files as empty."),

Re: diff-mode misinterprets empty lines.

[Jim Meyering]

Paul Eggert <eggert@CS.UCLA.EDU> wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Could you please revert this change?
>
> I did so, by installing the following patch into diffutils.  The old
> behavior (with trailing blanks) is now the default, and the new
> behavior (without them) is enabled only if the new
> --suppress-blank-empty option is given.
>
> However, the next POSIX version specifies a "diff -u" format in which
> those trailing blanks are optional, and apparently some "diff"
> implementations in the wild omit them even though no official or test
> version of GNU diff ever did.  So, as a practical matter, programs
> that parse "diff" output should not assume the trailing blanks are
> present, regardless of what GNU diff does.
>
> 2007-12-04  Paul Eggert  <eggert@cs.ucla.edu>
>
> 	* NEWS: New diff option --suppress-blank-empty (no longer -u default).
> 	* doc/diff.texi (Detailed Unified): Trailing spaces are no longer
> 	omitted.
> 	(Trailing Blanks): New section.
> 	(diff Options) Mention new option.
> 	* src/diff.h (suppress_blank_entry): New decl.
> 	* src/context.c (pr_unidiff_hunk): Support --suppress-blank-empty.
> 	* src/util.c (print_1_line): Likewise.
> 	* src/diff.c (longopts, main, option_help_msgid): Likewise.
> 	(SUPPRESS_BLANK_EMPTY_OPTION): New constant.

Hi Paul,

That looks fine.
Since I was thinking of using that new option always, I considered
changing the source to make the default in my private binary be to enable
the new option.  Since I'd rather not have to make private changes, what
do you think about having a compile-time option to change the default?
Not even a configure-time flag.

Since Emacs may eventually change to accept the new format, it may make
sense to change the default and to provide a --no-suppress-blank-empty
option.

Re: diff-mode misinterprets empty lines.

[David Kastrup]

Jim Meyering <jim@meyering.net> writes:

> Since I was thinking of using that new option always, I considered
> changing the source to make the default in my private binary be to
> enable the new option.  Since I'd rather not have to make private
> changes, what do you think about having a compile-time option to
> change the default?  Not even a configure-time flag.
>
> Since Emacs may eventually change to accept the new format, it may
> make sense to change the default and to provide a
> --no-suppress-blank-empty option.

No, it does not make sense to change the default.  Emacs is just one of
many tools processing diff output.  I remember gratuitous breakage of
git, too.  There are far too many tools relying on the _documented_ diff
output formats (please read
(info "(diff) Context")
for a detailed explanation of the diff formats) that it makes sense
breaking things all around for no tangible benefit.

I don't understand why this change was made in the first place, and I
don't understand why people would want to gratuitously make the format
less reliable to parse by tools, when the main usage _is_ the
application by independent tools.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: diff-mode misinterprets empty lines.

[Jim Meyering]

David Kastrup <dak@gnu.org> wrote:

> Jim Meyering <jim@meyering.net> writes:
>
>> Since I was thinking of using that new option always, I considered
>> changing the source to make the default in my private binary be to
>> enable the new option.  Since I'd rather not have to make private
>> changes, what do you think about having a compile-time option to
>> change the default?  Not even a configure-time flag.
>>
>> Since Emacs may eventually change to accept the new format, it may
>> make sense to change the default and to provide a
>> --no-suppress-blank-empty option.
>
> No, it does not make sense to change the default.  Emacs is just one of
> many tools processing diff output.  I remember gratuitous breakage of
> git, too.  There are far too many tools relying on the _documented_ diff
> output formats (please read
> (info "(diff) Context")

Hi David,

My sole interest is in the change to the *unidiff* format.
And that was not documented, back then.

> for a detailed explanation of the diff formats) that it makes sense
> breaking things all around for no tangible benefit.

The benefit was tangible enough to me to propose the change, and to Paul
to allow and extend it.  I'm sure you know that git tools have been
accepting the trailing-blank-free format for over a year, so they too
saw some benefit in accepting the new format.

Too many tools these days can automatically remove trailing blanks.
If I keep my code free of trailing blanks (and I do), those tools should
have no affect on my code.  I want the same benefit for diffs of my code.
IMHO, it's an improvement at least to allow a trailing-blank-free diff format.

> I don't understand why this change was made in the first place, and I
> don't understand why people would want to gratuitously make the format
> less reliable to parse by tools, when the main usage _is_ the
> application by independent tools.

You seem to underestimate Paul's concern for portability.  Git was young
at the time, and the format they cared about was unidiff, so it wasn't
*that* big a deal to fix it.  If we had known about the incompatibility
with diff-mode back then, I'm sure the new behavior would never have
been made the default.

Re: diff-mode misinterprets empty lines.

[David Kastrup]

Jim Meyering <jim@meyering.net> writes:

> David Kastrup <dak@gnu.org> wrote:
>
>> No, it does not make sense to change the default.  Emacs is just one of
>> many tools processing diff output.  I remember gratuitous breakage of
>> git, too.  There are far too many tools relying on the _documented_ diff
>> output formats (please read
>> (info "(diff) Context")
>
> My sole interest is in the change to the *unidiff* format.
> And that was not documented, back then.

Huh?  What makes you say that?

>> for a detailed explanation of the diff formats) that it makes sense
>> breaking things all around for no tangible benefit.
>
> The benefit was tangible enough to me to propose the change, and to
> Paul to allow and extend it.  I'm sure you know that git tools have
> been accepting the trailing-blank-free format for over a year, so they
> too saw some benefit in accepting the new format.

Huh?  What tangible benefit is "does not break in newer versions but
only gets less reliable"?

> Too many tools these days can automatically remove trailing blanks.

Why would you apply them to a _diff_?

> If I keep my code free of trailing blanks (and I do), those tools
> should have no affect on my code.

But your code is written as _diffs_.  diffs are _output_ formats, not
input formats.

> I want the same benefit for diffs of my code.

What benefit?

> IMHO, it's an improvement at least to allow a trailing-blank-free diff
> format.

We are not talking about whether it makes sense for Emacs diff to be
able to work around the output of broken file transfers.  We are talking
about whether it makes sense to break the output in the first place.
And a "trailing-blank-free diff format" gained in this manner is an
illusion, anyway, since diff must be able to represent lines differing
in trailing space.

>> I don't understand why this change was made in the first place, and I
>> don't understand why people would want to gratuitously make the
>> format less reliable to parse by tools, when the main usage _is_ the
>> application by independent tools.
>
> You seem to underestimate Paul's concern for portability.

Huh?  How does one gain portability by breaking output formats?

> Git was young at the time, and the format they cared about was
> unidiff, so it wasn't *that* big a deal to fix it.

Huh?  What do you mean by "fix it"?

> If we had known about the incompatibility with diff-mode back then,
> I'm sure the new behavior would never have been made the default.

We are not talking about an "incompatibility with diff-mode".  We are
talking about breaking the format specification.  That concerns _any_
tool reading the output of diff, not just diff-mode.

So again: where is the tangible benefit of breaking diff output in the
first place?  Because broken communication channels might break it, too?
What kind of benefit is it to prebreak it?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: diff-mode misinterprets empty lines.

[Paul Eggert]

David Kastrup <dak@gnu.org> writes:

>> Too many tools these days can automatically remove trailing blanks.
>
> Why would you apply them to a _diff_?

We're beating a dead horse here, since the default behavior has been
changed back, but since you're interested in doing an autopsy....

I regularly apply many tools to a diff.  I use Emacs, and other text
editors.  I send diffs via email, and I store them as part of other
text files.  The most recent email I sent to this forum contained a
diff, for example.

In environments that use diffs a lot, trailing white space can cause
problems.  Here's one example.  Emacs has a way of highlighting
trailing white space.  This feature is a nice thing to have in
general, since it highlights white space that in some applications can
cause harm (C source code, makefiles, etc.) and that the user normally
cannot see.  But if 'diff' output contains unnecessary trailing white
space, the highlighted white space turns into false alarms on your
screen.  In general, a false alarm is a bad thing: either it causes
you to turn off the alarm mechanism, or it causes you to ignore
similar alarms in the future.  So that is at least one good motivation
for suppressing unnecessary trailing white space.

I'm not saying that it is always the right thing to suppress trailing
blanks; I'm only saying that there are good reasons to do so in some
cases.

By the way, the diff that I sent in my previous email
<http://lists.gnu.org/archive/html/bug-gnu-utils/2007-12/msg00026.html>
suppressed the trailing white space in question.  Did you notice?
I've used this format for many months now, for patches that I send via
email.  No problems have been reported with these patches.  And I send
out my fair share of patches.

Re: diff-mode misinterprets empty lines.

[Juanma Barranquero]

On Dec 5, 2007 6:45 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:

> But if 'diff' output contains unnecessary trailing white
> space, the highlighted white space turns into false alarms on your
> screen.  In general, a false alarm is a bad thing: either it causes
> you to turn off the alarm mechanism, or it causes you to ignore
> similar alarms in the future.

Or it makes you add

 (add-hook 'diff-mode-hook (lambda () (setq show-trailing-whitespace nil)))

to your .emacs if it really bothers you. This is not "to turn off the
alarm mechanism", but rather than having it activated for patches is a
mistake, because patches are not text (as David has said).

> I've used this format for many months now, for patches that I send via
> email.  No problems have been reported with these patches.  And I send
> out my fair share of patches.

Again, I agree with David: I've installed quite a few patches, and
I've never reported back problems with them if I could trivially fix
them. Lack of complains is not a positive proof.

             Juanma

Re: diff-mode misinterprets empty lines.

[Stefan Monnier]

> Again, I agree with David: I've installed quite a few patches, and
> I've never reported back problems with them if I could trivially fix
> them. Lack of complains is not a positive proof.

Also Emacs's diff-mdoe already has code to try and fix missing trailing
whitespace (as well as a few other common problems introduced by brain
dead mailers).  This support was good enough when only occasional broken
patches had those problems.


        Stefan

Re: diff-mode misinterprets empty lines.

[Glenn Morris]

In an effort to clear FOR-RELEASE, here is a simple-minded attempt to
deal with this. It seems to fix the original problem at least.


*** diff-mode.el.~1.122.~	2007-10-21 13:00:12.000000000 -0700
--- diff-mode.el	2008-01-05 15:58:41.000000000 -0800
***************
*** 403,409 ****
    (setq style (diff-hunk-style style))
    (let ((end (and (re-search-forward (case style
  				       ;; A `unified' header is ambiguous.
! 				       (unified (concat "^[^-+# \\]\\|"
  							diff-file-header-re))
  				       (context "^[^-+#! \\]")
  				       (normal "^[^<>#\\]")
--- 403,409 ----
    (setq style (diff-hunk-style style))
    (let ((end (and (re-search-forward (case style
  				       ;; A `unified' header is ambiguous.
! 				       (unified (concat "^[^-+# \\\n]\\|"
  							diff-file-header-re))
  				       (context "^[^-+#! \\]")
  				       (normal "^[^<>#\\]")

Re: diff-mode misinterprets empty lines.

[Stefan Monnier]

> In an effort to clear FOR-RELEASE, here is a simple-minded attempt to
> deal with this. It seems to fix the original problem at least.

It is problematic when you try to apply a hunk via C-c C-a because any
empty line following your hunk (e.g. the last hunk in a C-x v =) will be
taken as being part of the hunk and C-c C-a will think that the original
text was just missing a newline and will add it (as a result of its
fuzzy matching feature).


        Stefan


> *** diff-mode.el.~1.122.~	2007-10-21 13:00:12.000000000 -0700
> --- diff-mode.el	2008-01-05 15:58:41.000000000 -0800
> ***************
> *** 403,409 ****
>     (setq style (diff-hunk-style style))
>     (let ((end (and (re-search-forward (case style
>   				       ;; A `unified' header is ambiguous.
> ! 				       (unified (concat "^[^-+# \\]\\|"
>   							diff-file-header-re))
>   				       (context "^[^-+#! \\]")
>   				       (normal "^[^<>#\\]")
> --- 403,409 ----
>     (setq style (diff-hunk-style style))
>     (let ((end (and (re-search-forward (case style
>   				       ;; A `unified' header is ambiguous.
> ! 				       (unified (concat "^[^-+# \\\n]\\|"
>   							diff-file-header-re))
>   				       (context "^[^-+#! \\]")
>   				       (normal "^[^<>#\\]")

Re: diff-mode misinterprets empty lines.

[Glenn Morris]

Stefan Monnier wrote:

>> In an effort to clear FOR-RELEASE, here is a simple-minded attempt to
>> deal with this. It seems to fix the original problem at least.
>
> It is problematic when you try to apply a hunk via C-c C-a because any
> empty line following your hunk (e.g. the last hunk in a C-x v =) will be
> taken as being part of the hunk and C-c C-a will think that the original
> text was just missing a newline and will add it (as a result of its
> fuzzy matching feature).

Oh dear. Would it suffice to back up over empty lines at the end of a
unified diff? Ie treat empty-lines as part of a hunk unless they are
at the very end of the diff?

Re: diff-mode misinterprets empty lines.

[Glenn Morris]

Glenn Morris wrote:

> Would it suffice to back up over empty lines at the end of a unified
> diff? Ie treat empty-lines as part of a hunk unless they are at the
> very end of the diff?

Sigh, that won't work in the CVS trunk, where the end of vc-diff
output looks like:

<last line of diff hunk>
<empty line>
<empty line>
Diffs between working revision and workfile end here.

I don't personally like that last line anyway...

Diffs between %s and %s end here (was: diff-mode misinterprets empty lines.)

[Reiner Steib]

On Mon, Jan 14 2008, Glenn Morris wrote:

> <last line of diff hunk>
> <empty line>
> <empty line>
> Diffs between working revision and workfile end here.
>
> I don't personally like that last line anyway...

Me too.  I'm still distracted by it.  What is the purpose of this
line?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Diffs between %s and %s end here

[Miles Bader]

Reiner Steib <reinersteib+gmane@imap.cc> writes:
>> <last line of diff hunk>
>> <empty line>
>> <empty line>
>> Diffs between working revision and workfile end here.
>>
>> I don't personally like that last line anyway...
>
> Me too.  I'm still distracted by it.  What is the purpose of this
> line?

I agree, the extra junk at the end is annoying.  Should be nuked.

-Miles

-- 
`To alcohol!  The cause of, and solution to,
 all of life's problems' --Homer J. Simpson

Re: Diffs between %s and %s end here

[Glenn Morris]

Miles Bader wrote:

> I agree, the extra junk at the end is annoying.  Should be nuked.

I'll remove it in a week or so if no-one objects.