bug#15535: 24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system

bug#15535: 24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system

[Dmitry Gutov]

Otherwise it's hard to find out which coding system was used, after all.

See here why it's useful:
http://lists.gnu.org/archive/html/emacs-devel/2013-10/msg00129.html

The following test passes in Emacs 24.3 but fails on trunk:

(ert-deftest save-buffer-updates-buffer-file-coding-system ()
  (let ((file (expand-file-name "foo" temporary-file-directory))
        (default-buffer-file-coding-system 'utf-8-unix))
    (unwind-protect
        (with-temp-buffer
          (insert "abcdef\n")
          (write-file file))
      (with-current-buffer (find-file-noselect file)
        (should (eq 'undecided (coding-system-change-eol-conversion
                                buffer-file-coding-system nil)))
        (insert "водка матрёшка селёдка")
        (save-buffer)
        ;; Fails here:
        (should (eq 'utf-8-unix buffer-file-coding-system)))
      (delete-file file))))

In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.6.4)
 of 2013-10-04 on axl
Bzr revision: 114513 eggert@cs.ucla.edu-20131003161631-vox3mdtalfjg13ed
Windowing system distributor `The X.Org Foundation', version 11.0.11303000
System Description:	Ubuntu 13.04

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Dmitry Gutov]

Sorry, here's a better test:

(ert-deftest save-buffer-updates-buffer-file-coding-system ()
   (let ((file (expand-file-name "foo" temporary-file-directory))
         (default-buffer-file-coding-system 'utf-8-unix))
     (find-file file)
     (insert "abcdef\n")
     (save-buffer)
     (kill-buffer)
     (unwind-protect
         (with-current-buffer (find-file-noselect file)
           (should (eq 'undecided (coding-system-change-eol-conversion
                                   buffer-file-coding-system nil)))
           (insert "водка матрёшка селёдка")
           (save-buffer)
           (let ((coding-system buffer-file-coding-system))
             (kill-buffer)
             (should (eq 'utf-8-unix coding-system))))
       (delete-file file))))

Likewise, succeeds on 24.3, fails on trunk.

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Eli Zaretskii]

(I've added Handa-san to this discussion, as I'm not sure I didn't miss
anything in looking into this.)

> Date: Sun, 06 Oct 2013 02:09:01 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> Sorry, here's a better test:
> 
> (ert-deftest save-buffer-updates-buffer-file-coding-system ()
>    (let ((file (expand-file-name "foo" temporary-file-directory))
>          (default-buffer-file-coding-system 'utf-8-unix))
>      (find-file file)
>      (insert "abcdef\n")
>      (save-buffer)
>      (kill-buffer)
>      (unwind-protect
>          (with-current-buffer (find-file-noselect file)
>            (should (eq 'undecided (coding-system-change-eol-conversion
>                                    buffer-file-coding-system nil)))
>            (insert "водка матрёшка селёдка")
>            (save-buffer)
>            (let ((coding-system buffer-file-coding-system))
>              (kill-buffer)
>              (should (eq 'utf-8-unix coding-system))))
>        (delete-file file))))
> 
> Likewise, succeeds on 24.3, fails on trunk.

Thanks.  For the record, a simpler test case is this:

 emacs -Q
 C-x C-f foo RET
 
Insert some ASCII text, then save the buffer, kill it, and visit the
file again:

 C-x C-s
 C-x k RET
 C-x C-f foo RET

You now have foo with `undecided' as its buffer-file-coding-system.
Then:

 C-u C-\ cyrillic-translit RET
 abvgde
 C-\
 C-x C-s

The file is saved (as UTF-8, as can be seen by examining it on disk),
but without asking for encoding, and without changing
buffer-file-coding-system to reflect the actual encoding.

What happens is that `undecided' silently encodes the buffer in UTF-8,
but never communicates that fact back to its callers.  So write-region
thinks it used `undecided', as does select-safe-coding-system.  The
latter is actually equipped to DTRT when the `prefer-utf-8' variant of
`undecided' is used, but that is not the case here.

Is this what was supposed to happen, or is something misbehaving here?

If the former, we could perhaps add some flag to struct undecided_spec
and set it whenever the encoder used by `undecided' sees a non-ASCII
character, and then use that flag to set last-coding-system-used to
UTF-8.  Does this make sense?

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Dmitry Gutov]

On 06.10.2013 19:51, Eli Zaretskii wrote:
> If the former, we could perhaps add some flag to struct undecided_spec
> and set it whenever the encoder used by `undecided' sees a non-ASCII
> character, and then use that flag to set last-coding-system-used to
> UTF-8.

That already happens (last-coding-system-used has the right value right 
after the file is written), but I don't think I can use it: even if 
`ruby-mode-set-encoding' is moved to after-save-hook, as long as it's 
not the first function in this hook (and I can't ensure that it is), the 
previous functions can also do some I/O and thus change 
last-coding-system-used's value.

And that the reason I reverted 114527 in 114533, which in turn sparked 
the discussion in emacs-devel.

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Eli Zaretskii]

> Date: Sun, 06 Oct 2013 23:58:57 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: 15535@debbugs.gnu.org
> 
> On 06.10.2013 19:51, Eli Zaretskii wrote:
> > If the former, we could perhaps add some flag to struct undecided_spec
> > and set it whenever the encoder used by `undecided' sees a non-ASCII
> > character, and then use that flag to set last-coding-system-used to
> > UTF-8.
> 
> That already happens (last-coding-system-used has the right value right 
> after the file is written)

Not here, it doesn't.  I see 'undecided'.  And that is part of the
problem.

> but I don't think I can use it: even if 
> `ruby-mode-set-encoding' is moved to after-save-hook, as long as it's 
> not the first function in this hook (and I can't ensure that it is), the 
> previous functions can also do some I/O and thus change 
> last-coding-system-used's value.

You can always take the value of last-coding-system-used as the first
thing you do.  The problem is that the value is wrong, at least in the
scenario I used to reproduce the problem.

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Dmitry Gutov]

On 07.10.2013 05:52, Eli Zaretskii wrote:
> Not here, it doesn't.  I see 'undecided'.  And that is part of the
> problem.

True, sorry. It worked for me previously, but I guess the value was 
similarly spoiled by some other function in after-save-hook.

>> `ruby-mode-set-encoding' is moved to after-save-hook, as long as it's
>> not the first function in this hook (and I can't ensure that it is), the
>> previous functions can also do some I/O and thus change
>> last-coding-system-used's value.
>
> You can always take the value of last-coding-system-used as the first
> thing you do.

If "I" am a function inside after-save-hook, I don't control the "first 
thing".

But now I see that `basic-save-buffer' does save the value of 
`last-coding-system-used' to either `save-buffer-coding-system' or 
`buffer-file-coding-system', depending on whether the former is non-nil.

So I can use those, and the problem is reduced to having the right 
`last-coding-system-used' value set.

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Kenichi Handa]

In article <83vc1a5omh.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
[...]
> You now have foo with `undecided' as its buffer-file-coding-system.
> Then:

>  C-u C-\ cyrillic-translit RET
>  abvgde
>  C-\
>  C-x C-s

> The file is saved (as UTF-8, as can be seen by examining it on disk),
> but without asking for encoding, and without changing
> buffer-file-coding-system to reflect the actual encoding.

> What happens is that `undecided' silently encodes the buffer in UTF-8,
> but never communicates that fact back to its callers.  So write-region
> thinks it used `undecided', as does select-safe-coding-system.  The
> latter is actually equipped to DTRT when the `prefer-utf-8' variant of
> `undecided' is used, but that is not the case here.

> Is this what was supposed to happen

No.  I think the behavior of 24.3 is correct.  So, some
change in trunk has a problem.  But, as far as I remember I
have not touched any codes that relate to this misbehavior.
I'm now investigating what has been changed from 24.3.

---
Kenichi Handa
handa@gnu.org

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Kenichi Handa]

In article <87siwd9lf6.fsf@gnu.org>, Kenichi Handa <handa@gnu.org> writes:

> No.  I think the behavior of 24.3 is correct.  So, some
> change in trunk has a problem.  But, as far as I remember I
> have not touched any codes that relate to this misbehavior.
> I'm now investigating what has been changed from 24.3.

Oops, it was me who enbugged...I had fixed the bug of 24.3
in a wrong way.  I've just committed the fix to trunk.

---
Kenichi Handa
handa@gnu.org

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Eli Zaretskii]

> From: Kenichi Handa <handa@gnu.org>
> Cc: eliz@gnu.org, 15535@debbugs.gnu.org, dgutov@yandex.ru
> Date: Sun, 13 Oct 2013 21:06:59 +0900
> 
> In article <87siwd9lf6.fsf@gnu.org>, Kenichi Handa <handa@gnu.org> writes:
> 
> > No.  I think the behavior of 24.3 is correct.  So, some
> > change in trunk has a problem.  But, as far as I remember I
> > have not touched any codes that relate to this misbehavior.
> > I'm now investigating what has been changed from 24.3.
> 
> Oops, it was me who enbugged...I had fixed the bug of 24.3
> in a wrong way.  I've just committed the fix to trunk.

Thanks, it works.

bug#15535: Acknowledgement (24.3.50; basic-save-buffer should update buffer-file-coding-system value if the contents were written using different coding system)

[Dmitry Gutov]

Version: 24.4

Fixed for me too, thanks.