Subject: Re: bug#19865: tar-untar-buffer: should honor default-directory

From: Eli Zaretskii <[email-masked]>
Date: Tue, 17 Feb 2015 17:46:16 +0200

> From: Ivan Shmakov <[email-masked]>
> Date: Tue, 17 Feb 2015 05:25:47 +0000
> 
> >> I agree that having to be careful in which buffer we are when we
>  >> read a given variable because it might be buffer-local is a source
>  >> of maintenance headaches, but we have that all over the place in
>  >> Elisp, and we don't really have any "better solution".
> 
>  > I don't really see a problem here that needs a solution.
> 
>         Yet at least two other Emacs users do.

That doesn't make the use case any less marginal, IMO.  You are both
power users who can easily get yourself out of this trouble.

Emacs maintenance shouldn't pay a price for marginal use cases for
which simple workarounds (like "M-x cd BACK") are available.

>  > A year from now no one will remember or understand why we use
>  > with-current-buffer in that place.  Doing so for such a weak reason
>  > is unwise, and no amount of cruft we have elsewhere can justify
>  > adding to that.
> 
>         We’re using with-current-buffer there for the sole reason that
>         write-region operates strictly on the current buffer.  It is so
>         without this change, and it remains so with it; the change
>         maintains this status quo just perfectly.  And as long as such
>         use /is/ acceptable in the current code, I fail to see why it
>         wouldn’t be in the replacement being discussed.

I already explained that, more than once: the change is obscure, and
is made for the sake of a marginal use case, where the user should
have returned to the original directory from which she cd'ed, to avoid
the problem.

>         What /is/ changed is that with-current-buffer will now go
>         immediately before write-region:
> 
>    (with-current-buffer data-buffer
>      (write-region …))
> 
>         Cf. the former:
> 
>    (with-current-buffer data-buffer
>      … lots of code to make sure the reader’s lost…
>              (write-region …))
> 
>         And at the same time:
> 
>    (let ((name (expand-file-name (the-name-of-the-archive-member))))
>      …)
> 
>         Versus the current:
> 
>    (let ((name (the-name-of-the-archive-member)))
>      …)

Now put yourself in the shoes of someone who needs to review this
change many moons from now, and try to imagine how "easy" it will be
for them to understand what the heck was that all about.

>         Please also note that the use of expand-file-name will still be
>         necessary if we decide to add an extra optional target-directory
>         argument to the function later, – and the /new/ code could
>         easily be changed to that effect, like:
>
>    (unless target-directory
>      (setq target-directory default-directory))
>    (let ((name (expand-file-name (the-name-of-the-archive-member)
>                                  target-directory)))
>      …)

When we have an explicit directory as an argument, expanding a file
name against that directory is something that is crystal-clear and
doesn't require any explanations.