bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Roland.Meier]

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

M-x sort-lines and M-x sort-fields always set the buffer modified
status ("-" -> "*" in column 5 of the status line), even if the region
was sorted and the command did not modify anything.
An unmodified buffer should stay unmodified if nothing was changed.
Reproduction:
C-x C-f a <return> a <return> b <return> c <return> C-x C-s <C-home> 
M-x s o r t - l <tab> <return>
Status line should start with --\---, it starts with --\**-


In GNU Emacs 23.1.1 (i386-mingw-nt5.1.2600)
 of 2009-07-30 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4)'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: DEU
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  show-paren-mode: t
  display-time-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t

Recent input:
C-s C-s C-s C-a <up> <up> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-up> 
<S-up> <S-up> <S-up> <S-up> <S-up> <S-up> <S-up> <S-up> 
<S-up> <S-up> <S-up> <S-up> <S-up> <S-up> <S-up> <S-up> 
<S-up> <S-up> <S-up> <S-up> <S-up> <S-down> <S-down> 
<S-down> <S-down> <S-down> <S-down> <S-down> <S-down> 
<S-down> <S-down> <S-delete> <S-up> <S-up> <S-up> <S-down> 
<S-delete> <delete> C-s C-s C-a <f11> C-s C-s C-s C-s 
C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s C-a <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <C-right> M-. <return> 
<lwindow> <help-echo> <help-echo> <help-echo> <lwindow> 
<f1> i <f2> <M-backspace> e t <tab> p r <tab> <return> 
<C-home> C-s s o r t C-s C-s C-a <C-home> <lwindow> 
<f3> <return> C-s s o r t C-s C-s C-a <C-home> <down> 
<down> <down> <down> 2 C-s C-s <down> <up> <return> 
C-s m o d i f C-s C-s C-s C-s C-a <lwindow> <f9> a 
<tab> <return> C-g <f2> a <return> a <return> b <return> 
c <return> <f3> <return> y e s <return> <f2> c : \ 
a <return> a <return> b <return> c <return> <f11> <C-S-home> 
<C-end> M-x s o r t - l <tab> <return> C-_ <lwindow> 
M-x r e p o <tab> r <tab> <return>

Recent messages:
Mark saved where search started
Mark set
Mark saved where search started [2 times]
Quit
(New file) [2 times]
Saving file c:/a...
Wrote c:/a
Mark set [3 times]
Undo!
Making completion list...

-- 
Mit freundlichen Grüßen
Roland Meier
     \|||/ 
     (o o) 
==ooO==U==Ooo== 
mailto:rm369@arcor.de

[-- Attachment #2: Type: text/html, Size: 8968 bytes --]

bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Stefan Monnier]

> M-x sort-lines and M-x sort-fields always set the buffer modified
> status ("-" -> "*" in column 5 of the status line), even if the region
> was sorted and the command did not modify anything.

Indeed.  The same holds true for fill-paragraph.

> An unmodified buffer should stay unmodified if nothing was changed.
> Reproduction:

Yes, that's generally desirable.  But in the above cases, given the way
the code currently works, it's fairly inconvenient to do (the code does
modify the buffer, it just so happens that the end text is the same as
the original text), so it doesn't seem worth the trouble.


        Stefan

bug#4597: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Roland.Meier]

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

> Yes, that's generally desirable.  But in the above cases, given the way
> the code currently works, it's fairly inconvenient to do (the code does
> modify the buffer, it just so happens that the end text is the same as
> the original text), so it doesn't seem worth the trouble.

Wouldn't it be possible in case of an unmodified buffer to copy the 
content of the region at the beginning to a temporary buffer, compare it 
to the result afterwards, and if they match to restore the unmodified 
status?

I sometimes need to check a list (which isn't small enough to be checked 
at a glance) after editing it if it is still sorted.
Now I write he region before and after sorting it to separate files and 
compare them, but I wonder if a powerful tool like emacs must keep such an 
obvious annoyance like this...

Thanks!
-- 
Mit freundlichen Grüßen
Roland Meier
     \|||/ 
     (o o) 
==ooO==U==Ooo== 

[-- Attachment #2: Type: text/html, Size: 1285 bytes --]

bug#4587: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Stefan Monnier]

>> Yes, that's generally desirable.  But in the above cases, given the way
>> the code currently works, it's fairly inconvenient to do (the code does
>> modify the buffer, it just so happens that the end text is the same as
>> the original text), so it doesn't seem worth the trouble.

> Wouldn't it be possible in case of an unmodified buffer to copy the
> content of the region at the beginning to a temporary buffer, compare it
> to the result afterwards, and if they match to restore the unmodified
> status?

I'd indeed expect that to implement the feature you request, the code
would have to do something like that.  Most likely not copying the text
itself, but instead storing an md5 or somesuch hash of the text.

> I sometimes need to check a list (which isn't small enough to be checked
> at a glance) after editing it if it is still sorted.
> Now I write he region before and after sorting it to separate files and
> compare them, but I wonder if a powerful tool like Emacs must keep such an
> obvious annoyance like this...

No, it definitely doesn't have to keep such obvious annoyances.
But it's not very high on the priority list.


        Stefan

bug#4597: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Magnus Henoch]

Roland.Meier@continental-corporation.com writes:

> I sometimes need to check a list (which isn't small enough to be checked 
> at a glance) after editing it if it is still sorted.

Maybe M-x diff-buffer-with-file could do that for you?

HTH,
Magnus

bug#4587: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Kevin Rodgers]

Stefan Monnier wrote:
>>> Yes, that's generally desirable.  But in the above cases, given the way
>>> the code currently works, it's fairly inconvenient to do (the code does
>>> modify the buffer, it just so happens that the end text is the same as
>>> the original text), so it doesn't seem worth the trouble.
> 
>> Wouldn't it be possible in case of an unmodified buffer to copy the
>> content of the region at the beginning to a temporary buffer, compare it
>> to the result afterwards, and if they match to restore the unmodified
>> status?
> 
> I'd indeed expect that to implement the feature you request, the code
> would have to do something like that.  Most likely not copying the text
> itself, but instead storing an md5 or somesuch hash of the text.

Not suitable for Emacs, but maybe useful for Roland:

(defadvice sort-lines (around restore-buffer-modified-p activate)
   (let* ((buffer-was-modified-p (buffer-modified-p))
	 (buffer-was-not-modified-md5 (if (not buffer-was-modified-p)
					  (md5 (current-buffer)))))
     ad-do-it
     (when (and (not buffer-was-modified-p)
	       (buffer-modified-p)
	       (not (equal buffer-was-not-modified-md5 (md5 (current-buffer)))))
       (restore-buffer-modified-p buffer-was-modified-p))))

>> I sometimes need to check a list (which isn't small enough to be checked
>> at a glance) after editing it if it is still sorted.
>> Now I write he region before and after sorting it to separate files and
>> compare them, but I wonder if a powerful tool like Emacs must keep such an
>> obvious annoyance like this...
> 
> No, it definitely doesn't have to keep such obvious annoyances.
> But it's not very high on the priority list.

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#4587: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Stefan Monnier]

>> I'd indeed expect that to implement the feature you request, the code
>> would have to do something like that.  Most likely not copying the text
>> itself, but instead storing an md5 or somesuch hash of the text.

> Not suitable for Emacs, but maybe useful for Roland:

> (defadvice sort-lines (around restore-buffer-modified-p activate)
>   (let* ((buffer-was-modified-p (buffer-modified-p))
> 	 (buffer-was-not-modified-md5 (if (not buffer-was-modified-p)
> 					  (md5 (current-buffer)))))
>     ad-do-it
>     (when (and (not buffer-was-modified-p)
> 	       (buffer-modified-p)
> 	       (not (equal buffer-was-not-modified-md5 (md5 (current-buffer)))))
>       (restore-buffer-modified-p buffer-was-modified-p))))

Maybe we could make it suitable, turn it into a macro and use it around
the various candidates.  AFAICT, here are the problems I see with it:
- the call to md5 should use as much as possible the internal encoding.
  I.e. at least pass an `emacs-internal' arg, tho it would be even
  better to let md5 work directly on the internal representation.
- it should only work on the afected region rather than the whole buffer
  (i.e. it needs start..end arguments).
- should it fiddle with the undo list?  or even revert the whole
  "without-effect" set of changes (the changes may result in the same
  final text, but they may very well have moved markers and changed
  text-properties, and it might be desirable to undo those changes, so
  as to better pretend nothing happened).


-- Stefan

bug#4587: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Kevin Rodgers]

Stefan Monnier wrote:
>>> I'd indeed expect that to implement the feature you request, the code
>>> would have to do something like that.  Most likely not copying the text
>>> itself, but instead storing an md5 or somesuch hash of the text.
> 
>> Not suitable for Emacs, but maybe useful for Roland:
> 
>> (defadvice sort-lines (around restore-buffer-modified-p activate)
>>   (let* ((buffer-was-modified-p (buffer-modified-p))
>> 	 (buffer-was-not-modified-md5 (if (not buffer-was-modified-p)
>> 					  (md5 (current-buffer)))))
>>     ad-do-it
>>     (when (and (not buffer-was-modified-p)
>> 	       (buffer-modified-p)
>> 	       (not (equal buffer-was-not-modified-md5 (md5 (current-buffer)))))
>>       (restore-buffer-modified-p buffer-was-modified-p))))
> 
> Maybe we could make it suitable, turn it into a macro and use it around
> the various candidates.  AFAICT, here are the problems I see with it:
> - the call to md5 should use as much as possible the internal encoding.
>   I.e. at least pass an `emacs-internal' arg, tho it would be even
>   better to let md5 work directly on the internal representation.
> - it should only work on the afected region rather than the whole buffer
>   (i.e. it needs start..end arguments).
> - should it fiddle with the undo list?  or even revert the whole
>   "without-effect" set of changes (the changes may result in the same
>   final text, but they may very well have moved markers and changed
>   text-properties, and it might be desirable to undo those changes, so
>   as to better pretend nothing happened).

Is this what you have in mind?

(defmacro with-maybe-region-modified (beg end &rest body)
   "Execute BODY, then `restore-buffer-modified-p' if the contents are unchanged.
BODY should not change the current buffer or modify the contents outside the region
between BEG and END."
   `(let* ((region-beg ,beg)
	  (region-end ,end)
	  (buffer-was-modified-p (buffer-modified-p))
	  (buffer-was-not-modified-md5 (if (not buffer-was-modified-p)
					   (md5 (current-buffer) region-beg region-end
						'emacs-mule)))
	  ;; (orig-buffer-undo-list buffer-undo-list)
	  (with-maybe-region-modified-result
	   (progn ,@body)))		; save-current-buffer?
      (when (and (not buffer-was-modified-p)
		(buffer-modified-p)
		(not (equal buffer-was-not-modified-md5
			    (md5 (current-buffer) region-beg region-end
				 'emacs-mule))))
        (restore-buffer-modified-p buffer-was-modified-p)
        ;; (setq buffer-undo-list orig-buffer-undo-list)
        )
      with-maybe-region-modified-result))


-- 
Kevin Rodgers
Denver, Colorado, USA

bug#4587: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Stefan Monnier]

>> Maybe we could make it suitable, turn it into a macro and use it around
>> the various candidates.  AFAICT, here are the problems I see with it:
>> - the call to md5 should use as much as possible the internal encoding.
>> I.e. at least pass an `emacs-internal' arg, tho it would be even
>> better to let md5 work directly on the internal representation.
>> - it should only work on the afected region rather than the whole buffer
>> (i.e. it needs start..end arguments).
>> - should it fiddle with the undo list?  or even revert the whole
>> "without-effect" set of changes (the changes may result in the same
>> final text, but they may very well have moved markers and changed
>> text-properties, and it might be desirable to undo those changes, so
>> as to better pretend nothing happened).

> Is this what you have in mind?

That looks almost right.  Here are some nitpicks:

> (defmacro with-maybe-region-modified (beg end &rest body)
>   "Execute BODY, then `restore-buffer-modified-p' if the contents are unchanged.
> BODY should not change the current buffer or modify the contents outside the region
> between BEG and END."

The docstring is wider then our convention.

>   `(let* ((region-beg ,beg)
> 	  (region-end ,end)
> 	  (buffer-was-modified-p (buffer-modified-p))
> 	  (buffer-was-not-modified-md5 (if (not buffer-was-modified-p)
> 					   (md5 (current-buffer) region-beg region-end
> 						'emacs-mule)))

Use `emacs-internal' here (in Emacs-23, the internal encoding is not
emacs-mule any more).

Don't use hardcoded symbols, since `body' might actually refer to
identically-named variables.

Add a FIXME-comment indicating that md5 should be improved to compute
this result without actually performing the encoding.  Or better yet,
provide a patch to `md5' which does just that.

> 	  ;; (orig-buffer-undo-list buffer-undo-list)
> 	  (with-maybe-region-modified-result
> 	   (progn ,@body)))		; save-current-buffer?

Yes, save-current-buffer seems to be necessary here, otherwise the code
will misbehave.

>      (when (and (not buffer-was-modified-p)
> 		(buffer-modified-p)
> 		(not (equal buffer-was-not-modified-md5
> 			    (md5 (current-buffer) region-beg region-end
> 				 'emacs-mule))))
>        (restore-buffer-modified-p buffer-was-modified-p)
>        ;; (setq buffer-undo-list orig-buffer-undo-list)
>        )
>      with-maybe-region-modified-result))

I think region-end should be assisted by a marker so we can detect if
the size of the region has changed and skip the second md5 call in
that case.

More importantly, I think the "(not (equal ...))" should be "(equal ...)".

Also, please add a FIXME-comment about whether we should maybe use
`undo' to revert the "effectless" changes.


        Stefan

bug#4587: bug#4597: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Lars Ingebrigtsen]

Roland.Meier@continental-corporation.com writes:

> M-x sort-lines and M-x sort-fields always set the buffer modified 
> status ("-" -> "*" in column 5 of the status line), even if the region 
> was sorted and the command did not modify anything. 
> An unmodified buffer should stay unmodified if nothing was changed. 

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I've now fixed this in Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4587: bug#4597: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Eli Zaretskii]

> Resent-From: Lars Ingebrigtsen <larsi@gnus.org>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs@gnu.org
> Resent-Sender: help-debbugs@gnu.org
> Cc: 4587@debbugs.gnu.org, rm369@arcor.de, 4597@debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 03 May 2022 21:24:09 +0200
> 
> Roland.Meier@continental-corporation.com writes:
> 
> > M-x sort-lines and M-x sort-fields always set the buffer modified 
> > status ("-" -> "*" in column 5 of the status line), even if the region 
> > was sorted and the command did not modify anything. 
> > An unmodified buffer should stay unmodified if nothing was changed. 
> 
> (I'm going through old bug reports that unfortunately weren't resolved
> at the time.)
> 
> I've now fixed this in Emacs 29.

This uses buffer-hash, which is only sensitive to changes in the byte
sequences of the buffer text.  AFAIU, it doesn't know about other
possible changes we perceive as "buffer changes", like changes in
faces, overlays, buffer-file-coding-system, etc.  Shouldn't this be
prominently documented in the macro's doc string?

bug#4597: Antwort: Re: bug#4587: 23.1; sort-lines and sort-fields always set buffer modified

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> This uses buffer-hash, which is only sensitive to changes in the byte
> sequences of the buffer text.  AFAIU, it doesn't know about other
> possible changes we perceive as "buffer changes", like changes in
> faces, overlays, buffer-file-coding-system, etc.  Shouldn't this be
> prominently documented in the macro's doc string?

(Adding overlays doesn't change modification status.)

If you think that needs to be spelled out, please go ahead, but it
doesn't seem necessary to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no