save-buffer in tar-mode

save-buffer in tar-mode

[Tak Ota]

Recently I realized that undo information was discarded as soon as
executing save-buffer in a buffer which was opened from tar-mode.
tar-subfile-save-buffer at a glance didn't lead me to exactly where it
discarded undo information.  Anyway, is this an intended behavior?
Normal save-buffer does not invalidate undo information.  It is quite
annoying losing ability to undo in this way.  I'm using the head of
CVS.

-Tak

Re: save-buffer in tar-mode

[Kenichi Handa]

In article <20040204.135033.207583862.Takaaki.Ota@am.sony.com>, Tak Ota <Takaaki.Ota@am.sony.com> writes:
> Recently I realized that undo information was discarded as soon as
> executing save-buffer in a buffer which was opened from tar-mode.
> tar-subfile-save-buffer at a glance didn't lead me to exactly where it
> discarded undo information.  Anyway, is this an intended behavior?

Yes.  tar-mode toggles enable-multibyte-characters on saving
a file by set-buffer-multibyte and that function discards
undo info.  To fix it, we must record this toggling in
buffer-undo-list.  I proposed it long ago but was rejected
at that time (I don't remember the reason, perhaps, just
because it's not worth to work on it).

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

Re: save-buffer in tar-mode

[Tak Ota]

Thu, 5 Feb 2004 08:36:51 +0900 (JST): Kenichi Handa <handa@m17n.org> wrote:

> In article <20040204.135033.207583862.Takaaki.Ota@am.sony.com>, Tak Ota <Takaaki.Ota@am.sony.com> writes:
> > Recently I realized that undo information was discarded as soon as
> > executing save-buffer in a buffer which was opened from tar-mode.
> > tar-subfile-save-buffer at a glance didn't lead me to exactly where it
> > discarded undo information.  Anyway, is this an intended behavior?
> 
> Yes.  tar-mode toggles enable-multibyte-characters on saving
> a file by set-buffer-multibyte and that function discards
> undo info.  To fix it, we must record this toggling in
> buffer-undo-list.  I proposed it long ago but was rejected
> at that time (I don't remember the reason, perhaps, just
> because it's not worth to work on it).

Thanks for the explanation.  I think it should be fixed.  Or, at least
this unwelcome side effect should be mentioned (warned) in the doc
string of set-buffer-multibyte where currently no mentioning about
this hidden trap.

-Tak

Re: save-buffer in tar-mode

[Kenichi Handa]

In article <20040204.155331.190222248.Takaaki.Ota@am.sony.com>, Tak Ota <Takaaki.Ota@am.sony.com> writes:
>>  Yes.  tar-mode toggles enable-multibyte-characters on saving
>>  a file by set-buffer-multibyte and that function discards
>>  undo info.  To fix it, we must record this toggling in
>>  buffer-undo-list.  I proposed it long ago but was rejected
>>  at that time (I don't remember the reason, perhaps, just
>>  because it's not worth to work on it).

> Thanks for the explanation.  I think it should be fixed.  Or, at least
> this unwelcome side effect should be mentioned (warned) in the doc
> string of set-buffer-multibyte where currently no mentioning about
> this hidden trap.

I added this sentence in the docstring.

If the multibyte flag was really changed, undo information of the
current buffer is cleared.

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

Re: save-buffer in tar-mode

[Stefan Monnier]

> If the multibyte flag was really changed, undo information of the
> current buffer is cleared.

Jonathan Yavner suggested to extend the undo log format with entries like
(function FUN . ARGS).  This would work wonders in this case.


        Stefan


PS: The only difficulty I see with the proposal is what to do with the
undo-in-region feature which generally needs to know which part of the
buffer is modified by a given undo item and should be able to update an
undo item so it applies to another part of the buffer (akin to updating
markers) so I suggested to turn it into (function FUN START LEN . ARGS).

Re: save-buffer in tar-mode

[Richard Stallman]

    Yes.  tar-mode toggles enable-multibyte-characters on saving
    a file by set-buffer-multibyte and that function discards
    undo info.  To fix it, we must record this toggling in
    buffer-undo-list.

That is one way to fix it.  Another way would be to avoid changing
enable-multibyte-characters.  The reason for doing so is to control
how insert-buffer copies the text.  This suggests that the cleanest
approach is to define a new primitive that does exactly the kind of
copying that is desired.  I think that should be easy.

Would you like to fix it that way?

I agree that it would be cleaner if set-buffer-multibyte did not
destroy the undo list.  In principle it certainly ought to preserve
the undo list.  But I think that could be a lot of work, too much
to be worth the trouble.

However, the judgment that it was too much trouble was in Emacs 20 or
so.  Since then we have made substantial changes in how multibyte
encoding works.  Maybe now it is not so hard.  By all means think
about it.

I don't think that the planned new extension to undo-list format will
help.  That could be useful for recording the set-buffer-multibyte
operation, so that it can be undone.  However, the real problem is to
update the existing undo-list entries so that they still make sense.
The new kind of undo entry won't help with that.

Meanwhile, even if set-buffer-multibyte is fixed to correct the undo
list, the other solution (a new copy operation) would be a better way
to do this operation.

Re: save-buffer in tar-mode

[Kenichi Handa]

In article <E1Aq7t5-0005ip-4d@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:

>     Yes.  tar-mode toggles enable-multibyte-characters on saving
>     a file by set-buffer-multibyte and that function discards
>     undo info.  To fix it, we must record this toggling in
>     buffer-undo-list.

> That is one way to fix it.  Another way would be to avoid changing
> enable-multibyte-characters.  The reason for doing so is to control
> how insert-buffer copies the text.  This suggests that the cleanest
> approach is to define a new primitive that does exactly the kind of
> copying that is desired.  I think that should be easy.

To avoid changing enable-multibyte-characters, I have
another idea.

Currently, on saving a file contained in a tar file, we
toggle multibyteness twice for both the file buffer and the
tar file buffer (very inefficient).  The reason for toggling
in the tar file buffer is that operations based on the tar
information work only in unibyte buffer.  And, why that
operations are so is that we don't have a primitive of
making buffer multibyte like the same way as
string-to-multibyte.  I proposed to have the same kind of
primitive for a buffer long ago, but at that time, it was
rejected.

I'd like to propose it again, i.e., make
set-buffer-multibyte accept `to' as FLAG.
(set-buffer-multibyte 'to) is exactly the same as:
(let ((str (string-to-multibyte (buffer-string))))
   (erase-buffer)
   (set-buffer-multibyte t)
   (insert str))     

Then, after we read the tar file by `no-conversion', we can
make the buffer multibyte by `to'.  After that, we never
have to change the multibyteness.

> I agree that it would be cleaner if set-buffer-multibyte did not
> destroy the undo list.  In principle it certainly ought to preserve
> the undo list.  But I think that could be a lot of work, too much
> to be worth the trouble.

> However, the judgment that it was too much trouble was in Emacs 20 or
> so.  Since then we have made substantial changes in how multibyte
> encoding works.  Maybe now it is not so hard.  By all means think
> about it.

> I don't think that the planned new extension to undo-list format will
> help.  That could be useful for recording the set-buffer-multibyte
> operation, so that it can be undone.  However, the real problem is to
> update the existing undo-list entries so that they still make sense.
> The new kind of undo entry won't help with that.

I don't understand why we have to update the existing
undo-list entries.  Each of them make sense in each
multibyte state.  So, as far as undo recovers multibyteness,
they always make sense when they are referred.

> Meanwhile, even if set-buffer-multibyte is fixed to correct the undo
> list, the other solution (a new copy operation) would be a better way
> to do this operation.

The new copy operation can be simulated by something like
this:

(defun new-insert-buffer (buf)
   (let ((str (save-execursion (set-buffer buf) (buffer-string))))
     (insert (if enable-multibyte-characters 
		 (string-as-multibyte str)
	       (string-as-unibyte str)))))

As it may be useful for the other case, I don't have strong
objection for having such primitive.  But, for tar-mode, I
think the one I proposed at the head is better.

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

Re: save-buffer in tar-mode

[Stefan Monnier]

> To avoid changing enable-multibyte-characters, I have
> another idea.

While we're throwing around ideas, I'd mention the one I had already
presented in the past: allow switching the buffer's text between
two buffers.
The idea is:
find-file loads the tar file's content into buffer A.

tar-mode is called which needs to show in buffer A the TOC (in multibyte
since it's made of chars).  Currently it does this by prepending the TOC and
using narrowing to hide the actual tar data (which really wants to be in
unibyte since it's made of bytes).  What tar-mode would ideally want to do
is copy the tar data to auxiliary buffer B so buffer A could be in multibyte
and buffer A in unibyte, but copying 25MB of tar data is a bit costly, so
instead we could offer a primitive that swaps the buffer_text of
two buffers.  Or something along these lines.


        Stefan

Re: save-buffer in tar-mode

[Richard Stallman]

      What tar-mode would ideally want to do
    > is copy the tar data to auxiliary buffer B so buffer A could be in multibyte
    > and buffer A in unibyte, but copying 25MB of tar data is a bit costly, so
    > instead we could offer a primitive that swaps the buffer_text of
    > two buffers.

I think it would work, and it might be a good idea.
However, something about it seems not quite right.
I have the feeling that the really right thing to do would
be different in some detail.

Perhaps attempting to write the buffer A into a file should actually
write buffer B into the file.

Re: save-buffer in tar-mode

[Stefan Monnier]

> I think it would work, and it might be a good idea.
> However, something about it seems not quite right.
> I have the feeling that the really right thing to do would
> be different in some detail.

Completely agreed, which is why I haven't hacked this feature.
I feel like it's likely to get into nasty issues with markers and internal
vars (like display vars and such) that would need to be
updated/invalidated.

The problem is:

- start with a big hunk of data in buffer A
- call tar-mode
- end with small TOC in buffer A and the big hunk of data in buffer B

Maybe a function `buffer-move' which does like `buffer-insert' except that
it can only insert into an empty buffer and that it deletes the text from
the source buffer.

But in any case, maybe this is just very silly and a plain copy of 25MB
would work just as well.  After all when we have to move the gap to update
this tar buffer, we also have to move a significant fraction of
those 25MB, so it won't make things much worse than they already are.


        Stefan

Re: save-buffer in tar-mode

[Richard Stallman]

    But in any case, maybe this is just very silly and a plain copy of 25MB
    would work just as well.  After all when we have to move the gap to update
    this tar buffer, we also have to move a significant fraction of
    those 25MB, so it won't make things much worse than they already are.

I would guess that it is quite rare to have to move the gap in the tar
buffer.  Most uses of tar mode are read-only, I would guess.
So that argument isn't really valid.

However, it may nonetheless be true that copying 25 MB is fast enough
nowadays.

Re: save-buffer in tar-mode

[Richard Stallman]

    I'd like to propose it again, i.e., make
    set-buffer-multibyte accept `to' as FLAG.
    (set-buffer-multibyte 'to) is exactly the same as:
    (let ((str (string-to-multibyte (buffer-string))))
       (erase-buffer)
       (set-buffer-multibyte t)
       (insert str))     

This approach is ok; it is simpler than what I proposed.
But this interface would be a kludge.  Please add a built-in
function with a new name to do this job.