all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#23824: 25.0.95; Prevent compare one buffer with itself
@ 2016-06-22 10:12 Tino Calancha
  2016-06-22 15:19 ` Eli Zaretskii
  2016-06-26  1:59 ` bug#23824: (no subject) Tino Calancha
  0 siblings, 2 replies; 8+ messages in thread
From: Tino Calancha @ 2016-06-22 10:12 UTC (permalink / raw)
  To: 23824


When the current buffer, buf-a, is visiting FILE-B, buf-b should
be a temporary buffer on sync with FILE-B.

./emacs -r -Q -eval '(progn (with-temp-file "/tmp/foo" (insert "foo")) 
(find-file "/tmp/foo") (insert "bar"))'
M-: (highlight-compare-with-file "/tmp/foo") RET
n n ; Answer no to saving suggestions.
;; Current buffer content different than /tmp/foo but no face 
highlight-changes shown.


In GNU Emacs 25.0.95.2 (x86_64-pc-linux-gnu, GTK+ Version 3.20.6)
Repository revision: 829733104db073f8abd67765eae162e7360281fa

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

From 47921b1f4898f9fad3acdaa15c2c70fa4b3f9061 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Wed, 22 Jun 2016 18:30:14 +0900
Subject: [PATCH] Prevent compare one buffer with itself

* lisp/hilit-chg.el (highlight-compare-with-file):
Use a new temporary buffer on sync with file-b; mark this
buffer as unmodified to prevent being prompt to save it (Bug#23824).
---
  lisp/hilit-chg.el | 14 +++++++++++---
  1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lisp/hilit-chg.el b/lisp/hilit-chg.el
index 8f042b6..89ca154 100644
--- a/lisp/hilit-chg.el
+++ b/lisp/hilit-chg.el
@@ -900,10 +900,18 @@ highlight-compare-with-file
    (let* ((buf-a (current-buffer))
  	 (file-a (buffer-file-name))
  	 (existing-buf (get-file-buffer file-b))
-	 (buf-b (or existing-buf
-		    (find-file-noselect file-b))))
+         (buf-b (cond ((eq buf-a existing-buf)
+                       (let ((buf-new (generate-new-buffer 
(generate-new-buffer-name
+                                                            (buffer-name 
buf-a)))))
+                         (with-current-buffer buf-new
+                           (insert-file-contents-literally file-b)
+                           (set-buffer-modified-p nil)) buf-new))
+                      (t
+                       (or existing-buf
+                           (find-file-noselect file-b))))))
      (highlight-markup-buffers buf-a file-a buf-b file-b (not 
existing-buf))
-    (unless existing-buf
+    (when (or (not existing-buf)
+              (eq buf-a existing-buf))
        (kill-buffer buf-b))
      ))

-- 
2.8.1







^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Prevent compare one buffer with itself
  2016-06-22 10:12 bug#23824: 25.0.95; Prevent compare one buffer with itself Tino Calancha
@ 2016-06-22 15:19 ` Eli Zaretskii
  2016-06-23  0:58   ` Tino Calancha
  2016-06-26  1:59 ` bug#23824: (no subject) Tino Calancha
  1 sibling, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2016-06-22 15:19 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 23824

> From: Tino Calancha <f92capac@gmail.com>
> Date: Wed, 22 Jun 2016 19:12:13 +0900 (JST)
> 
> When the current buffer, buf-a, is visiting FILE-B, buf-b should
> be a temporary buffer on sync with FILE-B.
> 
> ./emacs -r -Q -eval '(progn (with-temp-file "/tmp/foo" (insert "foo")) 
> (find-file "/tmp/foo") (insert "bar"))'
> M-: (highlight-compare-with-file "/tmp/foo") RET
> n n ; Answer no to saving suggestions.
> ;; Current buffer content different than /tmp/foo but no face 
> highlight-changes shown.

I think we need first to establish what exactly is the semantic of
this situation.  You are comparing a buffer with the file that the
buffer visits.  The doc string of this function tries to say something
about this situation:

  If the current buffer is visiting the file being compared against, it
  also will have its differences highlighted.  Otherwise, the file is
  read in temporarily but the buffer is deleted.

but I must confess that this is incomprehensible for me.  So I think
we should first establish what that means, or what the code is trying
to do.

> +                         (with-current-buffer buf-new
> +                           (insert-file-contents-literally file-b)

??? Why insert-file-contents-literally?  That definitely sounds wrong.

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Prevent compare one buffer with itself
  2016-06-22 15:19 ` Eli Zaretskii
@ 2016-06-23  0:58   ` Tino Calancha
  2016-06-23 15:27     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Tino Calancha @ 2016-06-23  0:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23824



On 06/23/2016 12:19 AM, Eli Zaretskii wrote:
>> From: Tino Calancha <f92capac@gmail.com>
>> Date: Wed, 22 Jun 2016 19:12:13 +0900 (JST)
>>
>> When the current buffer, buf-a, is visiting FILE-B, buf-b should
>> be a temporary buffer on sync with FILE-B.
>>
>> ./emacs -r -Q -eval '(progn (with-temp-file "/tmp/foo" (insert "foo"))
>> (find-file "/tmp/foo") (insert "bar"))'
>> M-: (highlight-compare-with-file "/tmp/foo") RET
>> n n ; Answer no to saving suggestions.
>> ;; Current buffer content different than /tmp/foo but no face
>> highlight-changes shown.
> I think we need first to establish what exactly is the semantic of
> this situation.  You are comparing a buffer with the file that the
> buffer visits.  The doc string of this function tries to say something
> about this situation:
>
>    If the current buffer is visiting the file being compared against, it
>    also will have its differences highlighted.  Otherwise, the file is
>    read in temporarily but the buffer is deleted.
>
> but I must confess that this is incomprehensible for me.  So I think
> we should first establish what that means, or what the code is trying
> to do.
  I understand what the doc means:  if the current buffer (buf-a) is 
visiting file-b,
then this func will perform a diff between buf-a and file-b.
* So, if  buf-a is modified, the command highlight you the differences 
with file-b, so
   let you decide if you want to save buf-a (overwritting file-b) or 
not.  It sounds useful.
* Current implementation doesn't match the doc string: even if buf-a is 
visiting file-b and
   modified, the func compare buf-a with buf-a, so that you never get 
nothing highlight
   in this case.

>> +                         (with-current-buffer buf-new
>> +                           (insert-file-contents-literally file-b)
> ??? Why insert-file-contents-literally?  That definitely sounds wrong.
>
> Thanks.

We can use: (insert-file-contents file-b)
it doesn't matter.  At the end, what this func does is comparing buffers
with ediff-diff-program so only the literal content would matter.






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Prevent compare one buffer with itself
  2016-06-23  0:58   ` Tino Calancha
@ 2016-06-23 15:27     ` Eli Zaretskii
  2016-06-24  5:07       ` Tino Calancha
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2016-06-23 15:27 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 23824

> Cc: 23824@debbugs.gnu.org
> From: Tino Calancha <f92capac@gmail.com>
> Date: Thu, 23 Jun 2016 09:58:02 +0900
> 
> > I think we need first to establish what exactly is the semantic of
> > this situation.  You are comparing a buffer with the file that the
> > buffer visits.  The doc string of this function tries to say something
> > about this situation:
> >
> >    If the current buffer is visiting the file being compared against, it
> >    also will have its differences highlighted.  Otherwise, the file is
> >    read in temporarily but the buffer is deleted.
> >
> > but I must confess that this is incomprehensible for me.  So I think
> > we should first establish what that means, or what the code is trying
> > to do.
>   I understand what the doc means:  if the current buffer (buf-a) is 
> visiting file-b,
> then this func will perform a diff between buf-a and file-b.

But then what is that "also" word doing in the doc string?

> * So, if  buf-a is modified, the command highlight you the differences 
> with file-b, so
>    let you decide if you want to save buf-a (overwritting file-b) or 
> not.  It sounds useful.
> * Current implementation doesn't match the doc string: even if buf-a is 
> visiting file-b and
>    modified, the func compare buf-a with buf-a, so that you never get 
> nothing highlight
>    in this case.

I think there's more here than meets the eye.  Did you ask yourself
why the user is asked twice whether to save the same buffer to the
same file in your scenario?  Why does it do that?  What does it have
in mind?

> >> +                         (with-current-buffer buf-new
> >> +                           (insert-file-contents-literally file-b)
> > ??? Why insert-file-contents-literally?  That definitely sounds wrong.
> >
> > Thanks.
> 
> We can use: (insert-file-contents file-b)
> it doesn't matter.

Oh, it matters a lot.  insert-file-contents-literally will bypass any
decoding and leave the CR-LF EOLs untranslated, something that you
don't want to affect the comparison.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Prevent compare one buffer with itself
  2016-06-23 15:27     ` Eli Zaretskii
@ 2016-06-24  5:07       ` Tino Calancha
  2016-06-24 13:16         ` bug#23824: 25.0.95; Do not prompt twice to save a buffer Tino Calancha
  0 siblings, 1 reply; 8+ messages in thread
From: Tino Calancha @ 2016-06-24  5:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tino Calancha, 23824


On Thu, 23 Jun 2016, Eli Zaretskii wrote:
>But then what is that "also" word doing in the doc string?
Nothing special:
It states that the person whom wrote this func took in account both cases:
*) When buf-a is not visiting file-b (let's call it 'default' case).
*) When buf-a is visiting file-b ('special' case).
Something like: "Dear doc readers, i let you know that i have _also_
considered the case when buf-a is visiting file-b".


>I think there's more here than meets the eye.  Did you ask yourself
>why the user is asked twice whether to save the same buffer to the
>same file in your scenario?  Why does it do that?  What does it have
>in mind?

I guess is just a bug in the logic.  The author of this code
overlooked that
(get-file-buffer file-b)
may return the very same buffer that buf-a (when buf-a is visiting 
file-b).
Then, calling
(highlight-markup-buffers BUF-A FILE-B BUF-A FILE-B)
will prompt you twice to save BUF-A when BUF-A is modified.
It prompts you to save buf-a again even if you saved buf-a after
the first prompt: this is because `highlight-markup-buffers'
save the bit on the modification status of BUF-A and BUF-B at the
top of the function.

In my patch, for this case (buf-a visiting file-b), i explicitely create a 
temporary buffer whose content equals file-b content.
Then, I call
(set-buffer-modified-p nil)
to prevent being prompted to save this temporary buffer.

>Oh, it matters a lot.  insert-file-contents-literally will bypass any
>decoding and leave the CR-LF EOLs untranslated, something that you
>don't want to affect the comparison.

I see.  Another example that reviewing a patch before applying it
is a good thing.
Then, my patch need to be modified:

insert-file-contents-literally -> insert-file-contents





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Do not prompt twice to save a buffer
  2016-06-24  5:07       ` Tino Calancha
@ 2016-06-24 13:16         ` Tino Calancha
  2016-06-25 10:26           ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Tino Calancha @ 2016-06-24 13:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: f92capac, 23824



On Fri, 24 Jun 2016, Tino Calancha wrote:

>(highlight-markup-buffers BUF-A FILE-B BUF-A FILE-B)
>will prompt you twice to save BUF-A when BUF-A is modified.
>It prompts you to save buf-a again even if you saved buf-a after
>the first prompt: this is because `highlight-markup-buffers'
>save the bit on the modification status of BUF-A and BUF-B at the
>top of the function.

User should be prompted just one in this case.
I propose following patch:

From 67eb8473757392f893c3f83227cbdfd184499e25 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Fri, 24 Jun 2016 21:53:56 +0900
Subject: [PATCH] Do not prompt twice to save a buffer

* lisp/hilit-chg.el (highlight-markup-buffers): (Bug#23824).
---
  lisp/hilit-chg.el | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/hilit-chg.el b/lisp/hilit-chg.el
index 8f042b6..d4276ce 100644
--- a/lisp/hilit-chg.el
+++ b/lisp/hilit-chg.el
@@ -782,7 +782,7 @@ highlight-markup-buffers
  	   a-start a-end len-a
  	   b-start b-end len-b
  	   (bufa-modified (buffer-modified-p buf-a))
-	   (bufb-modified (buffer-modified-p buf-b))
+	   (bufb-modified (and (not (eq buf-a buf-b)) (buffer-modified-p buf-b)))
  	   (buf-a-read-only (with-current-buffer buf-a buffer-read-only))
  	   (buf-b-read-only (with-current-buffer buf-b buffer-read-only))
  	   temp-a temp-b)
-- 
2.8.1







^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#23824: 25.0.95; Do not prompt twice to save a buffer
  2016-06-24 13:16         ` bug#23824: 25.0.95; Do not prompt twice to save a buffer Tino Calancha
@ 2016-06-25 10:26           ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2016-06-25 10:26 UTC (permalink / raw)
  To: Tino Calancha; +Cc: f92capac, 23824

> From: Tino Calancha <f92capac@gmail.com>
> Date: Fri, 24 Jun 2016 22:16:06 +0900 (JST)
> cc: f92capac@gmail.com, 23824@debbugs.gnu.org
> 
> >(highlight-markup-buffers BUF-A FILE-B BUF-A FILE-B)
> >will prompt you twice to save BUF-A when BUF-A is modified.
> >It prompts you to save buf-a again even if you saved buf-a after
> >the first prompt: this is because `highlight-markup-buffers'
> >save the bit on the modification status of BUF-A and BUF-B at the
> >top of the function.
> 
> User should be prompted just one in this case.
> I propose following patch:
> 
> >From 67eb8473757392f893c3f83227cbdfd184499e25 Mon Sep 17 00:00:00 2001
> From: Tino Calancha <f92capac@gmail.com>
> Date: Fri, 24 Jun 2016 21:53:56 +0900
> Subject: [PATCH] Do not prompt twice to save a buffer
> 
> * lisp/hilit-chg.el (highlight-markup-buffers): (Bug#23824).
> ---
>   lisp/hilit-chg.el | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lisp/hilit-chg.el b/lisp/hilit-chg.el
> index 8f042b6..d4276ce 100644
> --- a/lisp/hilit-chg.el
> +++ b/lisp/hilit-chg.el
> @@ -782,7 +782,7 @@ highlight-markup-buffers
>   	   a-start a-end len-a
>   	   b-start b-end len-b
>   	   (bufa-modified (buffer-modified-p buf-a))
> -	   (bufb-modified (buffer-modified-p buf-b))
> +	   (bufb-modified (and (not (eq buf-a buf-b)) (buffer-modified-p buf-b)))
>   	   (buf-a-read-only (with-current-buffer buf-a buffer-read-only))
>   	   (buf-b-read-only (with-current-buffer buf-b buffer-read-only))
>   	   temp-a temp-b)
> -- 
> 2.8.1

LGTM, thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23824: (no subject)
  2016-06-22 10:12 bug#23824: 25.0.95; Prevent compare one buffer with itself Tino Calancha
  2016-06-22 15:19 ` Eli Zaretskii
@ 2016-06-26  1:59 ` Tino Calancha
  1 sibling, 0 replies; 8+ messages in thread
From: Tino Calancha @ 2016-06-26  1:59 UTC (permalink / raw)
  To: 23824-done

Applied patch on master branch





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-06-26  1:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-22 10:12 bug#23824: 25.0.95; Prevent compare one buffer with itself Tino Calancha
2016-06-22 15:19 ` Eli Zaretskii
2016-06-23  0:58   ` Tino Calancha
2016-06-23 15:27     ` Eli Zaretskii
2016-06-24  5:07       ` Tino Calancha
2016-06-24 13:16         ` bug#23824: 25.0.95; Do not prompt twice to save a buffer Tino Calancha
2016-06-25 10:26           ` Eli Zaretskii
2016-06-26  1:59 ` bug#23824: (no subject) Tino Calancha

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.