RE: [Bug] Issues with format.el: coding system, byte/char confusi on

RE: [Bug] Issues with format.el: coding system, byte/char confusi on

[Wedler, Christoph]

Richard Stallman wrote:
 >>>> 1. Shouldn't `format-annotate-function' also save the buffer-local
 >>>> variable `buffer-file-coding-system' (as it does with
 >>>> `selective-display' and `enable-multibyte-characters' since v1.38)?

 > [...]

 > How about writing a patch to do this in general, and trying it out
 > for a while?  If it works well for you, we can install it.

	* format.el (format-annotate-function): Copy coding system into
	the new temp buffer.

- Christoph

diff -c format.el.~1.40~ format.el
*** format.el.~1.40~	Mon May  5 18:35:00 2003
--- format.el	Mon May  5 18:35:00 2003
***************
*** 223,232 ****
  	    (let ((copy-buf (get-buffer-create (format " *Format Temp %d*"
  						       format-count)))
  		  (sel-disp selective-display)
! 		  (multibyte enable-multibyte-characters))
  	      (with-current-buffer copy-buf
  		(setq selective-display sel-disp)
! 		(set-buffer-multibyte multibyte))
  	      (copy-to-buffer copy-buf from to)
  	      (set-buffer copy-buf)
  	      (format-insert-annotations write-region-annotations-so-far from)
--- 223,234 ----
  	    (let ((copy-buf (get-buffer-create (format " *Format Temp %d*"
  						       format-count)))
  		  (sel-disp selective-display)
! 		  (multibyte enable-multibyte-characters)
! 		  (coding-system buffer-file-coding-system))
  	      (with-current-buffer copy-buf
  		(setq selective-display sel-disp)
! 		(set-buffer-multibyte multibyte)
! 		(setq buffer-file-coding-system coding-system))
  	      (copy-to-buffer copy-buf from to)
  	      (set-buffer copy-buf)
  	      (format-insert-annotations write-region-annotations-so-far from)

RE: [Bug] Issues with format.el: coding system, byte/char confusi on

[Wedler, Christoph]

Richard Stallman wrote:
 >     (defun after-insert-file-set-buffer-file-coding-system (inserted)
 >        ...

 > If fixed this code, I think.  Does the version now installed seem
 > correct to you?

Yup, this seems to be correct to me.

Thanks for the fix.  The remaining issue of my original mail is:

Christoph Wedler wrote:
 >>> 1. Shouldn't `format-annotate-function' also save the buffer-local
 >>> variable `buffer-file-coding-system' (as it does with
 >>> `selective-display' and `enable-multibyte-characters' since v1.38)?

Richard Stallman wrote:
 >> That seems plausible, but one ought to test it.

Christoph Wedler wrote:
 > Currently, I do it in the format-encode function of my package X-Symbol
 > since July 2002 (it wouldn't work without), but I think this should be
 > done in general.

What do you think?

- Christoph

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Richard Stallman]

    Christoph Wedler wrote:
     >>> 1. Shouldn't `format-annotate-function' also save the buffer-local
     >>> variable `buffer-file-coding-system' (as it does with
     >>> `selective-display' and `enable-multibyte-characters' since v1.38)?

    Richard Stallman wrote:
     >> That seems plausible, but one ought to test it.

    Christoph Wedler wrote:
     > Currently, I do it in the format-encode function of my package X-Symbol
     > since July 2002 (it wouldn't work without), but I think this should be
     > done in general.

    What do you think?

How about writing a patch to do this in general, and trying it out
for a while?  If it works well for you, we can install it.

RE: [Bug] Issues with format.el: coding system, byte/char confusi on

[Wedler, Christoph]

Kenichi Handa wrote:

 > In article <67B8CED503F3D511BB9F0008C75DAD6605485571@dewdfx17>, "Wedler, Christoph" <christoph.wedler@sap.com> writes:

 >> I also don't think that the order is correct (if
 >> after-insert-file-set-buffer-file-coding-system doesn't do much, it
 >> might not matter): since the `buffer-file-coding-system' is used to save
 >> the RESULT of the format encode functions, it must be determined BEFORE
 >> the format-decode functions have been executed.

 > I agree.

 >>> Is there any real example where this question arises, or is it
 >>> purely hypothetical.

 >> I'm not sure since I don't know what
 >> `after-insert-file-set-buffer-file-coding-system' does exactly (is it
 >> more than eol-type handling?).

 > [...] So, as far as format-decode or any other functions in the
 > hook don't pay attention to buffer-file-coding-system

X-Symbol's decode function sometimes[1] does pay attention to
`buffer-file-coding-system'.  (A hypothetical question became real...)

 > or enable-multibyte-characters, we can do the task in any order.
 > But, conceptually it should be done before running any other Lisp
 > code.

It would be excellent if Emacs could be patched such that
`after-insert-file-set-buffer-file-coding-system' is called directly
from within Finsert_file_contents before calling Qformat_decode (instead
of adding this function to `after-insert-file-functions').

- Christoph

[1] with unique decoding and 8bit char use in the file: then \oe would
not be decoded with a latin9 coding system, but it would be decoded with
a latin1 coding system.

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Richard Stallman]

    It would be excellent if Emacs could be patched such that
    `after-insert-file-set-buffer-file-coding-system' is called directly
    from within Finsert_file_contents before calling Qformat_decode (instead
    of adding this function to `after-insert-file-functions').

I don't object.

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Kenichi Handa]

In article <E193VSw-0003VD-00@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:
>     It would be excellent if Emacs could be patched such that
>     `after-insert-file-set-buffer-file-coding-system' is called directly
>     from within Finsert_file_contents before calling Qformat_decode (instead
>     of adding this function to `after-insert-file-functions').

> I don't object.

Ok, I've just installed these changes.

2003-04-10  Kenichi Handa  <handa@m17n.org>

	* fileio.c (Vafter_insert_file_adjust_coding_function): New
	variable.
	(syms_of_fileio): Declare it as a lisp variable.
	(Finsert_file_contents): Call
	Vafter_insert_file_adjust_coding_function before calling
	decode-format.

2003-04-10  Kenichi Handa  <handa@m17n.org>

	* international/mule.el: Set
	after-insert-file-adjust-coding-function to
	after-insert-file-set-buffer-file-coding-system instead of adding
	it to after-insert-file-functions.

But, these names are very long.
	after-insert-file-adjust-coding-function
	after-insert-file-set-buffer-file-coding-system
Can't you think of shorter names?

---
Ken'ichi HANDA
handa@m17n.org

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Richard Stallman]

	    * fileio.c (Vafter_insert_file_adjust_coding_function): New
	    variable.

It doesn't need to be a Lisp variable.  It can call the function by
name using Qafter_insert_file_set_buffer_file_coding_system.

A shorter name could be after-insert-file-set-coding.

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Kenichi Handa]

In article <E193uFf-00046o-00@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:
> 	    * fileio.c (Vafter_insert_file_adjust_coding_function): New
> 	    variable.

> It doesn't need to be a Lisp variable.  It can call the function by
> name using Qafter_insert_file_set_buffer_file_coding_system.

> A shorter name could be after-insert-file-set-coding.

Ok, I've just fixed it as below:

2003-04-11  Kenichi Handa  <handa@m17n.org>

	* fileio.c (Vafter_insert_file_adjust_coding_function): Delete
	this variable.
	(Qafter_insert_file_set_coding): New variable.
	(syms_of_fileio): Initialize and staticpro it.  Delete declaration
	for after-insert-file-adjust-coding-function.
	(Finsert_file_contents): Call Qafter_insert_file_set_coding
	instead of Vafter_insert_file_adjust_coding_function.

2003-04-11  Kenichi Handa  <handa@m17n.org>

	* international/mule.el: Don't set
	after-insert-file-adjust-coding-function.
	(after-insert-file-set-coding): Renamed from
	after-insert-file-set-buffer-file-coding-system.

---
Ken'ichi HANDA
handa@m17n.org

RE: [Bug] Issues with format.el: coding system, byte/char confusi on

[Wedler, Christoph]

 >      1. Shouldn't `format-annotate-function' also save the buffer-local
 > 	variable `buffer-file-coding-system' (as it does with
 > 	`selective-display' and `enable-multibyte-characters' since v1.38)?

 > That seems plausible, but one ought to test it.

Currently, I do it in the format-encode function of my package X-Symbol
since July 2002 (it wouldn't work without), but I think this should be
done in general.

 >      2. To compute the coding system of the current buffer, Emacs adds
 > 	function `after-insert-file-set-buffer-file-coding-system' to
 > 	`after-insert-file-functions'.  Is this correct?

 > Yes.

 > 	 - decode functions using format.el,
 > 	 - decode functions in `after-insert-file-functions' before
 > 	   `after-insert-file-set-buffer-file-coding-system',
 > 	 - Emacs determines the coding system with
 > 	   `after-insert-file-set-buffer-file-coding-system',
 > 	 - decode functions in `after-insert-file-functions' after
 > 	   `after-insert-file-set-buffer-file-coding-system'

 > That would be true, if there is anything in
 > after-insert-file-functions other than
 > after-insert-file-set-buffer-file-coding-system.  Normally I believe
 > there isn't anything else.

What does this mean?  Is `after-insert-file-functions' obsolete without
being declared as such?

 > 	I would assume that the coding system must be determined before any
 > 	of the decode functions have been executed...

 > Not necessarily.  That would depend on what those other functions are.
 > Normally there aren't any others.  I suppose if you add other functions
 > to this hook that you should put them in the proper order.

As I said, the coding system is determined not only after some functions
in this hook, but also after the format-decode functions.

I also don't think that the order is correct (if
after-insert-file-set-buffer-file-coding-system doesn't do much, it
might not matter): since the `buffer-file-coding-system' is used to save
the RESULT of the format encode functions, it must be determined BEFORE
the format-decode functions have been executed.

 > Is there any real example where this question arises, or is it
 > purely hypothetical.

I'm not sure since I don't know what
`after-insert-file-set-buffer-file-coding-system' does exactly (is it
more than eol-type handling?).

 >      3. The docstring of `after-insert-file-functions' includes "It should
 > 	return the new byte count", whereas `format-decode' returns the
 > 	new char count.

 > It looks like they are really both measured in characters.
 > I will fix some doc strings.

In this case, you might also want to check (`position-bytes', but this might
be OK):

(defun after-insert-file-set-buffer-file-coding-system (inserted)
   ...
	      (let ((pos-byte (position-bytes (+ (point) inserted))))
		(set-buffer-multibyte nil)
		(setq inserted (- pos-byte (position-bytes (point))))))
	  (set-buffer-modified-p modified-p))))
  inserted)

- Christoph

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Kenichi Handa]

In article <67B8CED503F3D511BB9F0008C75DAD6605485571@dewdfx17>, "Wedler, Christoph" <christoph.wedler@sap.com> writes:
> I also don't think that the order is correct (if
> after-insert-file-set-buffer-file-coding-system doesn't do much, it
> might not matter): since the `buffer-file-coding-system' is used to save
> the RESULT of the format encode functions, it must be determined BEFORE
> the format-decode functions have been executed.

I agree.

>>  Is there any real example where this question arises, or is it
>>  purely hypothetical.

> I'm not sure since I don't know what
> `after-insert-file-set-buffer-file-coding-system' does exactly (is it
> more than eol-type handling?).

The task of that function are:

(1) Set buffer-file-coding-system according to the current
value and last-coding-system-used.

(2) If a file is visited by no-conversion or raw-text, set
the buffer unibyte.

So, as far as format-decode or any other functions in the
hook don't pay attention to buffer-file-coding-system or
enable-multibyte-characters, we can do the task in any
order.  But, conceptually it should be done before running
any other Lisp code.

---
Ken'ichi HANDA
handa@m17n.org

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Richard Stallman]

     > That would be true, if there is anything in
     > after-insert-file-functions other than
     > after-insert-file-set-buffer-file-coding-system.  Normally I believe
     > there isn't anything else.

    What does this mean?  Is `after-insert-file-functions' obsolete without
    being declared as such?

It is not obsolete.  What I am saying is that normally the only
element of after-insert-file-functions is
after-insert-file-set-buffer-file-coding-system.

If you add other elements, you can choose where to put them.

     > Not necessarily.  That would depend on what those other functions are.
     > Normally there aren't any others.  I suppose if you add other functions
     > to this hook that you should put them in the proper order.

    As I said, the coding system is determined not only after some functions
    in this hook, but also after the format-decode functions.

I think I am totally lost now.

Re: [Bug] Issues with format.el: coding system, byte/char confusi on

[Richard Stallman]

     >      3. The docstring of `after-insert-file-functions' includes "It should
     > 	return the new byte count", whereas `format-decode' returns the
     > 	new char count.

     > It looks like they are really both measured in characters.
     > I will fix some doc strings.

    In this case, you might also want to check (`position-bytes', but this might
    be OK):

    (defun after-insert-file-set-buffer-file-coding-system (inserted)
       ...
		  (let ((pos-byte (position-bytes (+ (point) inserted))))
		    (set-buffer-multibyte nil)
		    (setq inserted (- pos-byte (position-bytes (point))))))
	      (set-buffer-modified-p modified-p))))
      inserted)

If fixed this code, I think.  Does the version now installed seem
correct to you?