diff-mode misinterprets empty lines.

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.

[Richard Stallman]

    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.

Are you suggesting I should ask the diff maintainers to revert that
change?  That might be a good idea.

But in the mean time let's make Emacs work with the format generated
by diff 2.8.7.  Even if diff 2.8.8 undoes this change, 2.8.7 won't
disappear.  Would someone please do that and ack?

Re: diff-mode misinterprets empty lines.

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     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.
>
> Are you suggesting I should ask the diff maintainers to revert that
> change?  That might be a good idea.

Personally, I would consider this a good idea.  The "patch" program has
apparently been made robust in the presence of mail-mangled patches,
cut&paste carnage and other damage.  But "patch works with it" is, in my
opinion, not a positive definition of the diff format (while "patch
fails" is a different issue): patch is not the only program working with
diffs, and some version control systems have a diff-based workflow
without using "patch" for it.

Here is the format definition actually delivered with diff itself for
context diffs:

<URL:http://www.gnu.org/software/diffutils/manual/html_node/Detailed-Context.html#Detailed%20Context>

And here another for unified diffs:

<URL:http://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html#Detailed%20Unified>

It appears quite definite from the description that no whitespace
reduction is intended, neither for content nor for formatting
characters.

So yes, I would ask the diff maintainers to revert the change.  I can't
think of a good reason for it at all, but maybe they can, so it would
probably be a good idea to keep emacs-devel in the list (after all,
Emacs is one of the applications broken by this, for now): that way we
might together reach consensus on the best course.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: diff-mode misinterprets empty lines.

[Stefan Monnier]

>     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.

> Are you suggesting I should ask the diff maintainers to revert that
> change?  That might be a good idea.

Yes, please.


        Stefan

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.

[Andreas Schwab]

Jim Meyering <jim@meyering.net> writes:

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

(diff)Detailed Unified:

   The lines common to both files begin with a space character.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: diff-mode misinterprets empty lines.

[Jim Meyering]

Andreas Schwab <schwab@suse.de> wrote:
> Jim Meyering <jim@meyering.net> writes:
>
>> My sole interest is in the change to the *unidiff* format.
>> And that was not documented, back then.
>
> (diff)Detailed Unified:
>
>    The lines common to both files begin with a space character.

Oh.  So it did.

I suppose I was remembering that aspect was not specified by POSIX,
but now it is.

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.

[Paul Eggert]

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.

Could you instead use a shell script?

#!/bin/sh
exec /path/to/actual/diff/program --suppress-blank-empty "$@"

Re: diff-mode misinterprets empty lines.

[Jim Meyering]

Paul Eggert <eggert@CS.UCLA.EDU> 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.
>
> Could you instead use a shell script?
>
> #!/bin/sh
> exec /path/to/actual/diff/program --suppress-blank-empty "$@"

Of course :-)
Thanks.

Re: diff-mode misinterprets empty lines.

[David Kastrup]

Paul Eggert <eggert@CS.UCLA.EDU> writes:

> 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.

No.  In environments that use diffs, trailing white space needs to be
preserved.  Otherwise you could, for example, never have a diff that
removes trailing white space.

What _you_ mean is that in environments that _don't_ use diffs but
rather interpret them as naked text, inconveniences might occur.  But
those will occur with any patch dealing with trailing white space in any
manner, anyhow.

> 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.

Which only means that it is is wrong to interpret diff output with an
unmodified trailing whitespace detection mode.  Because diff files are
not merely text.  They carry a meaning.

> So that is at least one good motivation for suppressing unnecessary
> trailing white space.

The trailing white space is not unnecessary since it is part of the
format.

> 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.

diff-mode clearly is not such a one.

> 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 happen not to be a program.  If I had used Emacs for actually dealing
with the diff, I would have noticed.

Isn't it quite nonsensical to ask this question?  Would you be bothering
messing with trailing whitespace in the first place if it was
human-visible normally?

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

You mean, the mailing list thread on the Emacs developer list (which the
request to change this back originated on) is just a figment of my
imagination?  No problem report triggered it?

> And I send out my fair share of patches.

The "patch" program deals with this format violation.  Not everything
else does, and people often repair patches instead of reporting them.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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.

[Paul Eggert]

David Kastrup <dak@gnu.org> writes:

> In environments that use diffs, trailing white space needs to be
> preserved.  Otherwise you could, for example, never have a diff that
> removes trailing white space.

Yes, of course, but the trailing white space you're talking about is
unaffected by the change, so this is not the issue here.  We are
talking only about trailing white space generated by the diff format
itself, not about trailing white space in the data.

> it is is wrong to interpret diff output with an
> unmodified trailing whitespace detection mode.

Not if --suppress-blank-empty is used.  With that option, all the
trailing white space will be about changes to data, which is normally
something I want to see.  It's much easier to see changings that
affect only trailing white space that way.  That is an advantage of
suppressing the blanks in question.

If Emacs could be improved to mark only important trailing white
space; then --suppress-blank-empty would be less useful (at least,
less useful to Emacs users).  But I don't see how to do that in
general, when diffs are mixed in with other text.

> The trailing white space is not unnecessary since it is part of the
> format.

Not necessarily.  diff outputs several formats.  The definition of the
format is up to us, and depends on utility; it is not carved in stone.
Unless you count draft POSIX as the "stone"; in that case, the
trailing blanks of diff -u format are indeed unnecessary.

> Would you be bothering
> messing with trailing whitespace in the first place if it was
> human-visible normally?

Yes.  Human-visible trailing white space was one of the motivations
for this change.

Re: diff-mode misinterprets empty lines.

[Richard Stallman]

There is no need to argue.
We do need to change Emacs to cope if that space is missing.
And GNU diff should output the spaces by default as it did in the past.

Re: diff-mode misinterprets empty lines.

[Andreas Schwab]

Paul Eggert <eggert@CS.UCLA.EDU> writes:

> Not necessarily.  diff outputs several formats.  The definition of the
> format is up to us, and depends on utility; it is not carved in stone.

But was defined and documented several years ago.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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.

[Richard Stallman]

Thank you for working on this.
There are just a few bugs in FOR-RELEASE; with a little work
we can fix them all.

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: diff-mode misinterprets empty lines.

[Dan Nicolaescu]

Glenn Morris <rgm@gnu.org> writes:

  > 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...

I would vote to take out that line and the extra empty lines. 

They are annoying when you want to save the diff to a file and archive
it, or mail it to someone else, etc.

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.

Re: diff-mode misinterprets empty lines.

[Chong Yidong]

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

>> 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).

Is there any progress on this issue, or is help needed?

It seems to me that this is the last bug remaining the FOR-RELEASE,
although the release process is so opaque and confusing that I have
little idea what's going on.

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).

> Is there any progress on this issue, or is help needed?

No progress.  Help would be welcome.  I think it mostly involves trying
to use diff-sanity-checks (either the function itself or the ideas) at
more places.

E.g. diff-end-of-hunk should check that the place it found is consistent
with the hunk header's numbers and if not (and if looking at an empty
line) keep going further.

But be careful also that diff-end-of-hunk (and other functions) is/are
used in diff-fixup-modifs where the hunk header is presumed broken.

> It seems to me that this is the last bug remaining the FOR-RELEASE,
> although the release process is so opaque and confusing that I have
> little idea what's going on.

It was decided that this bug should not prevent the pretest.  I would
personally not hold the release for it either, so we already have
diff-sanity-checks which takes care of the most serious problems already.


        Stefan

Re: diff-mode misinterprets empty lines.

[Stefan Monnier]

> No progress.  Help would be welcome.  I think it mostly involves trying

Actually, as you may have seen, I've found the time to do it today,


        Stefan

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]

    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

[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

[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

[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

[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: 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

[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

[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

[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

[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.