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

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

[Ivan Shmakov]

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

Package:  emacs
Severity: minor
Tags: patch

	As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
	15:01:12 UTC), tar-untar-buffer uses the default-directory value
	for the “data” buffer (see tar-data-buffer), instead of the
	(arguably much less obscure) one for the tar-mode buffer itself
	(which could easily be changed by the user via M-x cd.)

	A possible fix is to remember the default-directory value in the
	same ‘let’ form used to do so for tar-parse-info, along the
	lines of the patch MIMEd.

-- 
FSF associate member #7257  np. A World Behind the World — Jami Sieber

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 864 bytes --]

--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -531,6 +542,7 @@ defun tar-untar-buffer ()
   (interactive)
   ;; FIXME: make it work even if we're not in tar-mode.
-  (let ((descriptors tar-parse-info))   ;Read the var in its buffer.
+  (let ((descriptors tar-parse-info)    ; Read the var in its buffer.
+        (dest-dir default-directory))
     (with-current-buffer
         (if (tar-data-swapped-p) tar-data-buffer (current-buffer))
       (set-buffer-multibyte nil)          ;Hopefully, a no-op.
@@ -544,5 +564,6 @@ defun tar-untar-buffer ()
                (dir (if (eq (tar-header-link-type descriptor) 5)
                         name
                       (file-name-directory name)))
+               (default-directory dest-dir)
                (start (tar-header-data-start descriptor))
                (end (+ start (tar-header-size descriptor))))

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 11:31:12 +0000
> 
> 	As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
> 	15:01:12 UTC), tar-untar-buffer uses the default-directory value
> 	for the “data” buffer (see tar-data-buffer), instead of the
> 	(arguably much less obscure) one for the tar-mode buffer itself
> 	(which could easily be changed by the user via M-x cd.)

Sorry, I don't understand the implications of the change, or even the
problem.  Could you elaborate?

The doc string of tar-untar-buffer says:

  "Extract all archive members in the tar-file into the current directory."

which seems to match what it does.  Why isn't default-directory of
tar-data-buffer the right choice for doing that?

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

[…]

 > The doc string of tar-untar-buffer says:

 >   "Extract all archive members in the tar-file into the current
 > directory."

 > which seems to match what it does.  Why isn't default-directory of
 > tar-data-buffer the right choice for doing that?

	Because the default-directory of the tar-mode buffer may be
	changed by the user (with M-x cd or otherwise) with the intent
	of directing tar-untar-buffer to extract the archive members
	into an arbitrary directory, per its docstring.

	Which fails, as the tar-data-buffer’s value for that same
	variable doesn’t get changed at that point.

	Another solution is to wrap with-current-buffer just around
	write-region, so that the rest of the code will still be
	evaluated with the tar-mode buffer being current.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 12:27:45 +0000
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> […]
> 
>  > The doc string of tar-untar-buffer says:
> 
>  >   "Extract all archive members in the tar-file into the current
>  > directory."
> 
>  > which seems to match what it does.  Why isn't default-directory of
>  > tar-data-buffer the right choice for doing that?
> 
> 	Because the default-directory of the tar-mode buffer may be
> 	changed by the user (with M-x cd or otherwise) with the intent
> 	of directing tar-untar-buffer to extract the archive members
> 	into an arbitrary directory, per its docstring.

Sounds like an obscure possibility to me.  If we want to give the user
an option to extract to a specific directory, why not make that
directory an argument of tar-untar-buffer, and prompt for it when the
function is called with a prefix argument?  That's how we usually let
the user change such defaults.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 12:27:45 +0000
>>>>> Eli Zaretskii <eliz@gnu.org> writes:

[…]

 >>> "Extract all archive members in the tar-file into the current
 >>> directory."

 >>> which seems to match what it does.  Why isn't default-directory of
 >>> tar-data-buffer the right choice for doing that?

 >> Because the default-directory of the tar-mode buffer may be changed
 >> by the user (with M-x cd or otherwise) with the intent of directing
 >> tar-untar-buffer to extract the archive members into an arbitrary
 >> directory, per its docstring.

 > Sounds like an obscure possibility to me.  If we want to give the
 > user an option to extract to a specific directory, why not make that
 > directory an argument of tar-untar-buffer, and prompt for it when the
 > function is called with a prefix argument?  That's how we usually let
 > the user change such defaults.

	No objection against such a new feature on my part.

	Still, the docstring mentions “current directory”, and it’s
	exactly the thing default-directory references and M-x cd sets.
	Thus the current behavior doesn’t match the documentation, so at
	least one of the two’s in the need for a fix.

-- 
FSF associate member #7257  np. El Dorado — Iron Maiden … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 12:47:48 +0000
> 
> 	Still, the docstring mentions “current directory”, and it’s
> 	exactly the thing default-directory references and M-x cd sets.
> 	Thus the current behavior doesn’t match the documentation

It does if you don't use "M-x cd".

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 12:47:48 +0000

 >> Still, the docstring mentions “current directory”, and it’s exactly
 >> the thing default-directory references and M-x cd sets.  Thus the
 >> current behavior doesn’t match the documentation

 > It does if you don't use "M-x cd".

	It doesn’t if you C-x C-w into a file in a different directory.

	Or is your point that tar-mode buffer’s default-directory should
	/not/ be honored, unless under some specific circumstances
	(as in: for a just loaded .tar file)?

-- 
FSF associate member #7257  np. Starblind — Iron Maiden … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 13:34:21 +0000
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> >>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 12:47:48 +0000
> 
>  >> Still, the docstring mentions “current directory”, and it’s exactly
>  >> the thing default-directory references and M-x cd sets.  Thus the
>  >> current behavior doesn’t match the documentation
> 
>  > It does if you don't use "M-x cd".
> 
> 	It doesn’t if you C-x C-w into a file in a different directory.
> 
> 	Or is your point that tar-mode buffer’s default-directory should
> 	/not/ be honored, unless under some specific circumstances
> 	(as in: for a just loaded .tar file)?

My point is that there is no reason to cater to such user behavior.

I agree that there should be a way for the user to extract into a
directory of her choice, but the way to give that capability is to let
the user explicitly request a non-default directory.  Once we have
that capability, any user who complains about the issue you wanted to
fix can be told "just invoke the function with an argument".

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 13:34:21 +0000

[…]

 >> It doesn’t if you C-x C-w into a file in a different directory.

 >> Or is your point that tar-mode buffer’s default-directory should
 >> /not/ be honored, unless under some specific circumstances (as in:
 >> for a just loaded .tar file)?

 > My point is that there is no reason to cater to such user behavior.

	That is: to the possible use of C-x C-w to save the (possibly
	modified) Tar archive to a new file in some arbitrary directory?

	Also to note is that thanks to the tar-new-entry command, it’s
	possible to create .tar archives entirely in Emacs.  Consider:

    C-x b *new* RET
    M-x tar-mode RET
    C-x C-q
    I newfile RET
    RET
    Hello, world!
    C-x C-s
    C-x b *new* RET
    C-x C-w /where/my/tar/files/are.tar RET

	Now, where do you think M-x tar-untar-buffer RET would unpack
	this newly created archive?

[…]

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Stefan Monnier]

> 	As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
> 	15:01:12 UTC), tar-untar-buffer uses the default-directory value
> 	for the “data” buffer (see tar-data-buffer), instead of the
> 	(arguably much less obscure) one for the tar-mode buffer itself
> 	(which could easily be changed by the user via M-x cd.)

I agre with Eli that it's a somewhat obscure corner case, but as someone
who does `M-x cd' fairly often for similar purposes, I think the patch
is right.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Sat, 14 Feb 2015 09:49:50 -0500
> Cc: 19865@debbugs.gnu.org
> 
> > 	As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
> > 	15:01:12 UTC), tar-untar-buffer uses the default-directory value
> > 	for the “data” buffer (see tar-data-buffer), instead of the
> > 	(arguably much less obscure) one for the tar-mode buffer itself
> > 	(which could easily be changed by the user via M-x cd.)
> 
> I agre with Eli that it's a somewhat obscure corner case, but as someone
> who does `M-x cd' fairly often for similar purposes, I think the patch
> is right.

So we now need someone third to resolve the tie.

Any opinions out there?

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

[Stefan Monnier]

> So we now need someone third to resolve the tie.

I don't see why you object: the command is executed by the user in one
buffer, and it just so happens that its implementation switches
to some internal auxiliary buffer.  The value of `default-directory'
that should be used is the one that the user knows about, not the one
kept by the hidden buffer, over which the user has no control.

This behavior is an unintended result of my change in tar-mode where we
now have 2 buffers (rather than keeping all the raw tar data in the same
buffer as the pretty list, hidden via narrowing).


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: ivan@siamics.net, 19865@debbugs.gnu.org
> Date: Sun, 15 Feb 2015 20:43:17 -0500
> 
> I don't see why you object

I can only re-iterate what I already said: we shouldn't cater to
marginal use cases like that with code that is "tricky" (a.k.a.
"maintenance headache").  People who change directories of their
buffers should (and do) know what they are doing.  If doing that
causes them annoyances, they will know better next time.

> the command is executed by the user in one
> buffer, and it just so happens that its implementation switches
> to some internal auxiliary buffer.  The value of `default-directory'
> that should be used is the one that the user knows about, not the one
> kept by the hidden buffer, over which the user has no control.

Are we still talking about the situation where a user did "M-x cd"?
Or is this another use case?  If the latter, please describe it in its
entirety.  If the former, then is there still a problem if the user
refrains from "M-x cd"?

> This behavior is an unintended result of my change in tar-mode where we
> now have 2 buffers (rather than keeping all the raw tar data in the same
> buffer as the pretty list, hidden via narrowing).

Understood, but unintended results do not necessarily need fixes, just
because they are unintended.  The important question is: what, if any,
real problems are caused as unintended results?  We are discussing
those problems, so the fact that they are unintended results doesn't
seem important to me.

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

[Stefan Monnier]

> I can only re-iterate what I already said: we shouldn't cater to
> marginal use cases like that with code that is "tricky" (a.k.a.
> "maintenance headache").  People who change directories of their
> buffers should (and do) know what they are doing.  If doing that
> causes them annoyances, they will know better next time.

Hmm... so you're considering `M-x cd' as harmful?
I'm surprised, it looks like a perfectly normal command to me.
If we don't want normal users to use, we should at least
`disable-command' and/or rename it to something more verbose and
less attractive.

>> the command is executed by the user in one buffer, and it just so
>> happens that its implementation switches to some internal auxiliary
>> buffer.  The value of `default-directory' that should be used is the
>> one that the user knows about, not the one kept by the hidden buffer,
>> over which the user has no control.
> Are we still talking about the situation where a user did "M-x cd"?

Actually, no, I'm talking about how the code *should* behave from a simple
semantic correctness point of view.
This then appears in cases such as `M-x cd' indeed, but it might appear
in other cases I haven't thought about.

> If the former, then is there still a problem if the user refrains from
> "M-x cd"?

Until we decide to deprecate `M-x cd' I think this question is not
really relevant.

> Understood, but unintended results do not necessarily need fixes, just
> because they are unintended.  The important question is: what, if any,
> real problems are caused as unintended results?  We are discussing
> those problems, so the fact that they are unintended results doesn't
> seem important to me.

The patch he provides fixes the immediate problem, which in my book is
a plain bug (maybe for a "corner case", but still a plain bug), but in
terms of "disagrees with the docstring" and "disagrees with my mental
model of what is right".
Also the fix doesn't make the code more obscure or more complex.

Hence I still fail to see why you're opposing it.

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".


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: ivan@siamics.net, 19865@debbugs.gnu.org
> Date: Mon, 16 Feb 2015 14:34:02 -0500
> 
> > I can only re-iterate what I already said: we shouldn't cater to
> > marginal use cases like that with code that is "tricky" (a.k.a.
> > "maintenance headache").  People who change directories of their
> > buffers should (and do) know what they are doing.  If doing that
> > causes them annoyances, they will know better next time.
> 
> Hmm... so you're considering `M-x cd' as harmful?

Not harmful, potentially dangerous, especially in buffers that have
associated files, or where default-directory plays some other
significant role.  People who do that should "M-x cd" back before they
invoke functions that use the value.

It's the same as working in the shell: if you changed a directory, you
need to go back before you operate on files, or risk operate on wrong
files.

> 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.  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.

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

[Stefan Monnier]

> It's the same as working in the shell:

That's also my impression.

> if you changed a directory, you need to go back before you operate on
> files, or risk operate on wrong files.

No: I changed directory specifically because I wanted to operate on
other files.  And I often don't go back.

>> 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.  A year from
> now no one will remember or understand why we use with-current-buffer
> in that place.

The comment explains why and the comment will still be there a year
from now.

The code takes a file name from one buffer and uses it in another
buffer: every time we do it, we have to be extra careful to make sure
the file name is absolute, and that's what the patch does.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: ivan@siamics.net, 19865@debbugs.gnu.org
> Date: Mon, 16 Feb 2015 18:40:51 -0500
> 
> > if you changed a directory, you need to go back before you operate on
> > files, or risk operate on wrong files.
> 
> No: I changed directory specifically because I wanted to operate on
> other files.  And I often don't go back.

Then your next delete command will probably delete wrong files.

> > I don't really see a problem here that needs a solution.  A year from
> > now no one will remember or understand why we use with-current-buffer
> > in that place.
> 
> The comment explains why and the comment will still be there a year
> from now.

Comments cannot be a replacement for clear, self-explanatory code that
has no hidden dependencies.

> The code takes a file name from one buffer and uses it in another
> buffer: every time we do it, we have to be extra careful to make sure
> the file name is absolute, and that's what the patch does.

If both buffers has the same default-directory, as they should,
there's no need to do anything special.

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

[Stefan Monnier]

>> No: I changed directory specifically because I wanted to operate on
>> other files.  And I often don't go back.
> Then your next delete command will probably delete wrong files.

No: it would delete the wrong file if I had *not* changed directory.

> Comments cannot be a replacement for clear, self-explanatory code that
> has no hidden dependencies.

Of course, but in the case under discussion there is such a dependency,
and we're just considering two different versions of the code, both of
which have the exact same hidden dependency.
IOW this situation sucks, but the patch is not the cause of it and
doesn't make it worse (it makes it marginally better by adding
a comment and by making the behavior right).

>> The code takes a file name from one buffer and uses it in another
>> buffer: every time we do it, we have to be extra careful to make sure
>> the file name is absolute, and that's what the patch does.
> If both buffers has the same default-directory, as they should,
> there's no need to do anything special.

Right: *as they should*.  But there's no mechanism to keep them in sync.
So the next best thing is to make sure we only use the default-directory
from the proper buffer.

> To solve this problem, I proposed, in the beginning of this thread, to
> allow the user to specify a directory where to extract the tarball, by
> invoking the command with a prefix argument.

Your proposal is fine, but is orthogonal to the patch proposed by Ivan.


        Stefan

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

[Wolfgang Jenkner]

On Mon, Feb 16 2015, Stefan Monnier wrote:

> [Eli wrote:]
>> It's the same as working in the shell:
>
> That's also my impression.

The shell analogy might be a bit misleading and is perhaps the reason
why you seem to be talking past each other: Personally, at the shell
command line, I don't change to the directory holding the tarball,
instead I change to the directory where I want to extract it, like this

$ cd ~/src; tar xvf ~/Downloads/foo.tar.gz

This is quite different from the situation in emacs where

C-x C-f ~/Downloads/foo.tar.gz

results in buffers whose default-directory is ~/Downloads, hence the
reason why doing M-x cd ~/src in the non-hidden buffer should have the
effect as in the proposed patch.

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

[Eli Zaretskii]

> From: Wolfgang Jenkner <wjenkner@inode.at>
> Cc: Eli Zaretskii <eliz@gnu.org>,  ivan@siamics.net,  19865@debbugs.gnu.org
> Date: Tue, 17 Feb 2015 18:03:26 +0100
> 
> The shell analogy might be a bit misleading and is perhaps the reason
> why you seem to be talking past each other: Personally, at the shell
> command line, I don't change to the directory holding the tarball,
> instead I change to the directory where I want to extract it, like this
> 
> $ cd ~/src; tar xvf ~/Downloads/foo.tar.gz
> 
> This is quite different from the situation in emacs where
> 
> C-x C-f ~/Downloads/foo.tar.gz
> 
> results in buffers whose default-directory is ~/Downloads, hence the
> reason why doing M-x cd ~/src in the non-hidden buffer should have the
> effect as in the proposed patch.

To solve this problem, I proposed, in the beginning of this thread, to
allow the user to specify a directory where to extract the tarball, by
invoking the command with a prefix argument.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Stefan Monnier  Date: Mon, 16 Feb 2015 14:34:02 -0500

[…]

 >> 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.

 > 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.

	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)))
     …)

	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)))
     …)

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> 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.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Tue, 17 Feb 2015 05:25:47 +0000

[…]

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

	It is not a workaround for the problem in question.

[…]

 >> 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:

	And I’ve already provided an example where default-directory is
	changed by Emacs behind the back of the user.

 > the change is obscure,

	Yet it makes the code /less/ obscure that it currently is.

 > and is made for the sake of a marginal use case,

	It is made for the sake of consistency with the rest of the
	tar-mode.el code.  And sure: it fixes that marginal use case at
	the same time, too.

 > where the user should have returned to the original directory from
 > which she cd'ed, to avoid the problem.

	The use of M-x cd on a buffer does not influence the behavior of
	tar-untar-buffer for that same buffer in any way.  When it
	thinks – for whatever reason – that it should use
	/that/directory as its target, – it’s stuck that way, and no
	sensible user action is going to change its twisted mind.
	(Incidentally, this is the very description of this bug.)

	There’s simply /no/ problem which the user could solve by
	changing the value of default-directory – whether back, forth,
	or sideways.

 >> 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 …))

[…]

 > 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.

	Whose shoes do you think I was in while I’ve investigated this
	issue?  My first thought: why on Earth does with-current-buffer
	wraps some 18 LoC while it’s needed for exactly a single one?

	As for the “many moons” part, I think I’d request a re-review
	after a few.  Hopefully someone’d provide some more convincing
	arguments by then, since I’m pretty much out of ink right now.

	(And TIA to that party, sure.)

[…]

-- 
FSF associate member #7257  tar-mode.el does nasty things 3013 B6A0 230E 334A

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

[Ivan Shmakov]

>>>>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

 >> As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
 >> 15:01:12 UTC), tar-untar-buffer uses the default-directory value for
 >> the “data” buffer (see tar-data-buffer), instead of the (arguably
 >> much less obscure) one for the tar-mode buffer itself (which could
 >> easily be changed by the user via M-x cd.)

 > I agree with Eli that it's a somewhat obscure corner case,

	(Which makes me wonder if using write-file on a tar-mode buffer
	a similarly obscure corner case?)

 > but as someone who does `M-x cd' fairly often for similar purposes, I
 > think the patch is right.

	Avoiding switching buffers until around the actual write-region
	call may still be a better alternative (as that should make
	mistakes of this kind hardier to introduce), but results in a
	lengthier patch.  Any opinion on which way I should go there?

	Unless there be objections, I hope to push a fix for this issue
	within the next two days.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Ivan Shmakov]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

>>>>> Ivan Shmakov <ivan@siamics.net> writes:

[…]

 > Avoiding switching buffers until around the actual write-region call
 > may still be a better alternative (as that should make mistakes of
 > this kind hardier to introduce), but results in a lengthier patch.
 > Any opinion on which way I should go there?

	Per my reading of the code, with-current-buffer is generally
	used in tar-mode.el around the smallest fragments possible.

	Please thus consider the revised patch MIMEd, which I’ve tried
	to make consistent with such an approach.

	Also to note is that the awareness of the general “data buffer
	default-directory” issue dates back to 2001 at the least
	(considering the excerpt below, for instance), so I don’t seem
	to understand how making tar-untar-buffer consistent with the
	rest of the tar-mode.el code could ever be harmful?

	TIA.

commit e8421604cdd386af0c32fb7cf698882ec6b74015
Author: Gerd Moellmann <gerd@gnu.org>
Date:   2001-08-07 13:36:14 +0000

    (tar-extract): Avoid generating a new buffer
    for each file visited.  From Markus Rost <rost@math.ohio-state.edu>.

$ git archive --format=tar  e8421604cdd3 -- tar-mode.el | tar -xO | nl -ba 
…
   749			;; Set the default-directory to the dir of the
   750			;; superior buffer. 
   751			(setq default-directory
   752			      (save-excursion
   753				(set-buffer tar-buffer)
   754				default-directory))
…
$ 

-- 
FSF associate member #7257  np. Meditation — David Modica 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 2390 bytes --]

diff --git a/lisp/tar-mode.el b/lisp/tar-mode.el
index 6c7f755..c6eef01 100644
--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -531,25 +531,28 @@ defun tar-untar-buffer ()
   "Extract all archive members in the tar-file into the current directory."
   (interactive)
   ;; FIXME: make it work even if we're not in tar-mode.
-  (let ((descriptors tar-parse-info))   ;Read the var in its buffer.
-    (with-current-buffer
-        (if (tar-data-swapped-p) tar-data-buffer (current-buffer))
-      (set-buffer-multibyte nil)          ;Hopefully, a no-op.
-      (dolist (descriptor descriptors)
-        (let* ((name (tar-header-name descriptor))
-               (dir (if (eq (tar-header-link-type descriptor) 5)
-                        name
-                      (file-name-directory name)))
-               (start (tar-header-data-start descriptor))
-               (end (+ start (tar-header-size descriptor))))
-          (unless (file-directory-p name)
-            (message "Extracting %s" name)
-            (if (and dir (not (file-exists-p dir)))
-                (make-directory dir t))
-            (unless (file-directory-p name)
-	      (let ((coding-system-for-write 'no-conversion))
-		(write-region start end name)))
-            (set-file-modes name (tar-header-mode descriptor))))))))
+  (let ((data-buf (if (tar-data-swapped-p) tar-data-buffer (current-buffer))))
+    (with-current-buffer data-buf
+      (set-buffer-multibyte nil))       ; Hopefully, a no-op.
+    (dolist (descriptor tar-parse-info)
+      (let* ((name (tar-header-name descriptor))
+	     (dir (if (eq (tar-header-link-type descriptor) 5)
+		      name
+		    (file-name-directory name)))
+	     (start (tar-header-data-start descriptor))
+	     (end (+ start (tar-header-size descriptor))))
+	(unless (file-directory-p name)
+	  (message "Extracting %s" name)
+	  (if (and dir (not (file-exists-p dir)))
+	      (make-directory dir t))
+	  (unless (file-directory-p name)
+	    (let ((coding-system-for-write 'no-conversion)
+		  ;; Note that default-directory may have a different
+		  ;; value in the data buffer.
+		  (name (expand-file-name name default-directory)))
+	      (with-current-buffer data-buf
+		(write-region start end name))))
+	  (set-file-modes name (tar-header-mode descriptor)))))))
 
 (defun tar-summarize-buffer ()
   "Parse the contents of the tar file in the current buffer."

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

[Stefan Monnier]

> 	Per my reading of the code, with-current-buffer is generally
> 	used in tar-mode.el around the smallest fragments possible.

Indeed.

> 	Please thus consider the revised patch MIMEd, which I’ve tried
> 	to make consistent with such an approach.

I think it's better, indeed.

BTW, the (set-buffer-multibyte nil) should probably just be remove (or
replaced by a cl-assert)


        Stefan

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

[Ivan Shmakov]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

>>>>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

[…]

 >> Please thus consider the revised patch MIMEd, which I’ve tried to
 >> make consistent with such an approach.

 > I think it's better, indeed.

	I’ve revised the change to apply default-directory just once
	and use the result consistently throughout the function.

 > BTW, the (set-buffer-multibyte nil) should probably just be remove
 > (or replaced by a cl-assert)

	Done.

Fix default-directory handling in tar-untar-buffer.

* lisp/tar-mode.el (tar-untar-buffer): Fixed: use the value of
default-directory local to the tar-mode buffer (was:
tar-data-buffer); clean-up.
    
Fixes: debbugs:19865

PS.  Should the pending objections be resolved by sometime around
	18:00 UTC today, I’d happily push the change to ‘master’.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 2333 bytes --]

--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -533,27 +533,32 @@
 
 (defun tar-untar-buffer ()
   "Extract all archive members in the tar-file into the current directory."
   (interactive)
   ;; FIXME: make it work even if we're not in tar-mode.
-  (let ((descriptors tar-parse-info))   ;Read the var in its buffer.
-    (with-current-buffer
-        (if (tar-data-swapped-p) tar-data-buffer (current-buffer))
-      (set-buffer-multibyte nil)          ;Hopefully, a no-op.
-      (dolist (descriptor descriptors)
-        (let* ((name (tar-header-name descriptor))
-               (dir (if (eq (tar-header-link-type descriptor) 5)
-                        name
-                      (file-name-directory name)))
-               (start (tar-header-data-start descriptor))
-               (end (+ start (tar-header-size descriptor))))
-          (unless (file-directory-p name)
-            (message "Extracting %s" name)
-            (if (and dir (not (file-exists-p dir)))
-                (make-directory dir t))
-            (unless (file-directory-p name)
+  (let ((data-buf (if (tar-data-swapped-p) tar-data-buffer
+                    (current-buffer))))
+    (with-current-buffer data-buf
+      (cl-assert (not enable-multibyte-characters)))
+    (dolist (descriptor tar-parse-info)
+      (let* ((orig (tar-header-name descriptor))
+	     ;; Note that default-directory may have different values
+	     ;; in the tar-mode and data buffers, so we stick to the
+	     ;; absolute file name from now on.
+	     (name (expand-file-name orig))
+	     (dir (if (eq (tar-header-link-type descriptor) 5)
+		      name
+		    (file-name-directory name)))
+	     (start (tar-header-data-start descriptor))
+	     (end (+ start (tar-header-size descriptor))))
+	(unless (file-directory-p name)
+	  (message "Extracting %s" orig)
+	  (if (and dir (not (file-exists-p dir)))
+	      (make-directory dir t))
+	  (unless (file-directory-p name)
+	    (with-current-buffer data-buf
 	      (let ((coding-system-for-write 'no-conversion))
-		(write-region start end name)))
-            (set-file-modes name (tar-header-mode descriptor))))))))
+		(write-region start end name))))
+	  (set-file-modes name (tar-header-mode descriptor)))))))
 
 (defun tar-summarize-buffer ()
   "Parse the contents of the tar file in the current buffer."

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

[Stefan Monnier]

> PS.  Should the pending objections be resolved by sometime around
> 	18:00 UTC today, I’d happily push the change to ‘master’.

Most of the change (tho not the removal of set-buffer-multibyte)
probably belongs in emacs-24 rather than in master.

> +    (with-current-buffer data-buf
> +      (cl-assert (not enable-multibyte-characters)))

Why not move this further down where you already do the
"(with-current-buffer data-buf"?


        Stefan

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

[Ivan Shmakov]

>>>>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

 >> PS.  Should the pending objections be resolved by sometime around
 >> 18:00 UTC today, I’d happily push the change to ‘master’.

 > Most of the change (tho not the removal of set-buffer-multibyte)
 > probably belongs in emacs-24 rather than in master.

	I certainly could push it to the stable branch, but as for
	actually /testing/ it there – that’d double the work for me
	(as I do not currently follow emacs-24 development.)

 >> +    (with-current-buffer data-buf
 >> +      (cl-assert (not enable-multibyte-characters)))

 > Why not move this further down where you already do the
 > "(with-current-buffer data-buf"?

	My only concern is that that latter with-current-buffer happens
	within a loop, and I don’t seem to see a reason for checking
	this condition for every archive member extracted.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Stefan Monnier]

> 	I certainly could push it to the stable branch, but as for
> 	actually /testing/ it there – that’d double the work for me
> 	(as I do not currently follow emacs-24 development.)

OK, your call.

>>> +    (with-current-buffer data-buf
>>> +      (cl-assert (not enable-multibyte-characters)))
>> Why not move this further down where you already do the
>> "(with-current-buffer data-buf"?
> 	My only concern is that that latter with-current-buffer happens
> 	within a loop, and I don’t seem to see a reason for checking
> 	this condition for every archive member extracted.

This assertion should soon be removed altogether anyway, so let's not
waste too much time with it: either way is fine.


        Stefan

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

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> Fix default-directory handling in tar-untar-buffer.
>
> * lisp/tar-mode.el (tar-untar-buffer): Fixed: use the value of
> default-directory local to the tar-mode buffer (was:
> tar-data-buffer); clean-up.
>
> Fixes: debbugs:19865
>
> PS.  Should the pending objections be resolved by sometime around
> 	18:00 UTC today, I’d happily push the change to ‘master’.

As far as I can tell, the patch was never applied?  It looks like a
correct change to me...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ivan Shmakov <ivan@siamics.net> writes:
>
>> Fix default-directory handling in tar-untar-buffer.
>>
>> * lisp/tar-mode.el (tar-untar-buffer): Fixed: use the value of
>> default-directory local to the tar-mode buffer (was:
>> tar-data-buffer); clean-up.
>>
>> Fixes: debbugs:19865
>>
>> PS.  Should the pending objections be resolved by sometime around
>> 	18:00 UTC today, I’d happily push the change to ‘master’.
>
> As far as I can tell, the patch was never applied?  It looks like a
> correct change to me...

That was three years ago, and now the patch didn't apply any more.

But I respun the change that Ivan had done and I've now applied it to
tar-mode.  I've tested that it now untars to the current directory (even
after M-x cd).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Ivan Shmakov]

	Well, let’s see if I can reword the report.

	As currently implemented (tested on 7eb2077f9dd1, 2015-02-06
	15:01:12 UTC), the *user* tar-untar-buffer command uses the
	value of the default-directory *user* variable, as set for the
	*internal* (tar-data-buffer) buffer – in place of the value that
	same variable has in the buffer /the command is invoked from./

	To stress it out: this report is /not/ intended to be a request
	for a feature for unpacking Tar archives into arbitrary
	directories, but is rather meant to point out that as currently
	implemented, the tar-untar-buffer behavior is *ill-defined*;
	especially if used after write-file (C-x C-w), save-buffer
	(C-x C-s; when a Tar archive is created from scratch), or
	M-x cd RET.  (As all these commands /may/ change the value of
	default-directory for the user interaction buffer, but do /not/
	do the same for the *internal* tar-data-buffer one.)

	I believe that using the values the *user* variables posses in
	*internal* buffers, when the command in question is invoked from
	a buffer dedicated to the *user* interaction and implemented by
	that same facility, is generally a wrong thing to do, and has a
	potential for confusion of users and developers alike.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 15:57:36 +0000
> 
> 	Well, let’s see if I can reword the report.

If all you want is for me to stop bothering you with comments, all you
need is just ask.  There's no need to have the bug reports reworded
around my comments.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 15:57:36 +0000

 >> Well, let’s see if I can reword the report.

 > If all you want is for me to stop bothering you with comments, all
 > you need is just ask.  There's no need to have the bug reports
 > reworded around my comments.

	All I want is to get the issue (that is: the misbehavior of
	tar-untar-buffer with respect to the tar-mode buffer value of
	default-directory) fixed.

	From where I stand, the shortest way there seems to get either
	of my two patches approved (that is: at least a single OK and no
	NAKs), so that I can safely push it.  If there’s any other way I
	should be aware of – I’d like to hear about it.

	My understanding is that the Stefan’s comment is an OK (for a
	former patch), while your request for “someone third to resolve
	the tie” is a NAK (please correct me if it’s not.)

	I appreciate the comments regarding the possible new “explicit
	destination directory” feature, and I guess I can work on a
	possible implementation once /this/ issue – and also #19481 –
	are fixed.  However, I believe that such a new feature is no
	substitute for a proper fix to this bug.

	Thanks again for your participation in this discussion.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 17:32:59 +0000
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> >>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 15:57:36 +0000
> 
>  >> Well, let’s see if I can reword the report.
> 
>  > If all you want is for me to stop bothering you with comments, all
>  > you need is just ask.  There's no need to have the bug reports
>  > reworded around my comments.
> 
> 	All I want is to get the issue (that is: the misbehavior of
> 	tar-untar-buffer with respect to the tar-mode buffer value of
> 	default-directory) fixed.
> 
> 	From where I stand, the shortest way there seems to get either
> 	of my two patches approved (that is: at least a single OK and no
> 	NAKs), so that I can safely push it.  If there’s any other way I
> 	should be aware of – I’d like to hear about it.
> 
> 	My understanding is that the Stefan’s comment is an OK (for a
> 	former patch), while your request for “someone third to resolve
> 	the tie” is a NAK (please correct me if it’s not.)

It is only a NAK if it's respected.  I cannot force anyone here to do
anything, I don;t have that power.

> 	I appreciate the comments regarding the possible new “explicit
> 	destination directory” feature, and I guess I can work on a
> 	possible implementation once /this/ issue – and also #19481 –
> 	are fixed.  However, I believe that such a new feature is no
> 	substitute for a proper fix to this bug.

I obviously disagree, but whatever.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 17:32:59 +0000

[…]

 >> My understanding is that the Stefan’s comment is an OK (for a former
 >> patch), while your request for “someone third to resolve the tie” is
 >> a NAK (please correct me if it’s not.)

 > It is only a NAK if it's respected.

	I will surely respect any NAKs from those in charge of the given
	package, or Emacs as a whole (as per the Savannah project page.)

 > I cannot force anyone here to do anything, I don’t have that power.

	We’re all volunteers here, yes.  But you can – and, I’d argue,
	should – /prevent/ the use of the Emacs community resources for
	the purposes contrary to the advancement of the project itself.
	(FWIW, it’s just the thing I do at Russian Wikibooks.)  And as
	long as there’s a NAK from you, I will consider my change more
	or less a matter of my taste, not something valuable to the
	Emacs community at large.

	Similarly, as long as there’re any objections from Lars
	regarding my SHR- or EWW-related suggestions, I’d rather start
	my own fork than push a single change.

 >> I appreciate the comments regarding the possible new “explicit
 >> destination directory” feature, and I guess I can work on a possible
 >> implementation once /this/ issue – and also #19481 – are fixed.
 >> However, I believe that such a new feature is no substitute for a
 >> proper fix to this bug.

 > I obviously disagree, but whatever.

	And I’d still like to hear why you think that tar-untar-buffer
	/must/ use the value of default-directory local to a buffer
	/other/ than the one the user called this command from.

	TIA.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 18:12:47 +0000
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> >>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 17:32:59 +0000
> 
> […]
> 
>  >> My understanding is that the Stefan’s comment is an OK (for a former
>  >> patch), while your request for “someone third to resolve the tie” is
>  >> a NAK (please correct me if it’s not.)
> 
>  > It is only a NAK if it's respected.
> 
> 	I will surely respect any NAKs from those in charge of the given
> 	package, or Emacs as a whole (as per the Savannah project page.)

The file says emacs-devel is in charge, so I'm not sure who you mean
here.

>  > I cannot force anyone here to do anything, I don’t have that power.
> 
> 	We’re all volunteers here, yes.  But you can – and, I’d argue,
> 	should – /prevent/ the use of the Emacs community resources for
> 	the purposes contrary to the advancement of the project itself.

I don't know what that means in practice.  I have my opinions and
views, but who is to say whether they are or aren't contrary to the
advancement?

>  >> I appreciate the comments regarding the possible new “explicit
>  >> destination directory” feature, and I guess I can work on a possible
>  >> implementation once /this/ issue – and also #19481 – are fixed.
>  >> However, I believe that such a new feature is no substitute for a
>  >> proper fix to this bug.
> 
>  > I obviously disagree, but whatever.
> 
> 	And I’d still like to hear why you think that tar-untar-buffer
> 	/must/ use the value of default-directory local to a buffer
> 	/other/ than the one the user called this command from.

Because it's simpler, and makes the code easier to follow.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 18:12:47 +0000
>>>>> Eli Zaretskii <eliz@gnu.org> writes:

[…]

 >> I will surely respect any NAKs from those in charge of the given
 >> package, or Emacs as a whole (as per the Savannah project page.)

 > The file says emacs-devel is in charge, so I'm not sure who you mean
 > here.

--cut: https://savannah.gnu.org/projects/emacs/ --
   Project Admins:
     - Stefan Monnier
     - Eli Zaretskii
     - Richard M. Stallman
     - Miles Bader
     - Francesco Potortì
     - Kim F. Storm
--cut: https://savannah.gnu.org/projects/emacs/ --

 >>> I cannot force anyone here to do anything, I don’t have that power.

 >> We’re all volunteers here, yes.  But you can – and, I’d argue,
 >> should – /prevent/ the use of the Emacs community resources for the
 >> purposes contrary to the advancement of the project itself.

 > I don't know what that means in practice.

	In practice, that means that those who keep interfering with the
	community efforts risk losing their commit access.  (Or “posting
	access” to the mailing list, just like it happened to one of the
	bug-hurd@ participants something like a decade back.  Etc.)

 > I have my opinions and views, but who is to say whether they are or
 > aren't contrary to the advancement?

	It’s up to those who’re in charge to decide.  I can understand
	the lack of interest in holding such a responsibility, but the
	problem is: I don’t want it, either.

	Still, I have no means to /force/ you to review my submissions.
	But once you did – I won’t use community resources against your
	/objections./  (As opposed to, say, lack of appreciation – I can
	accept that perfectly well.)

	(In short: the golden rule of volunteer-driven projects is that
	the leaders tell us not what we /do,/ but what we /do not./)

[…]

 >> And I’d still like to hear why you think that tar-untar-buffer
 >> /must/ use the value of default-directory local to a buffer /other/
 >> than the one the user called this command from.

 > Because it's simpler, and makes the code easier to follow.

	My former patch expands the code by a single line; my latter one
	adds two more for a comment (while removing one level of nesting
	from a fair chunk of the code at the same time.)  How is that
	making it any harder to follow?

	Besides, as I’ve already noted, it does make tar-untar-buffer
	more consistent with both the rest of tar-mode.el, /and/ the
	rest of Emacs.  I thus believe that a single extra LoC is fully
	justified here.

-- 
FSF associate member #7257  np. Cherry Blossom — David Modica  B6A0 230E 334A

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

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 14 Feb 2015 19:12:04 +0000
> 
>  >> And I’d still like to hear why you think that tar-untar-buffer
>  >> /must/ use the value of default-directory local to a buffer /other/
>  >> than the one the user called this command from.
> 
>  > Because it's simpler, and makes the code easier to follow.
> 
> 	My former patch expands the code by a single line; my latter one
> 	adds two more for a comment (while removing one level of nesting
> 	from a fair chunk of the code at the same time.)  How is that
> 	making it any harder to follow?

Length of code is only one factor, insignificant in this case.  Subtle
dependencies between different parts of the code, as well as
overriding the default-directory value, is what complicates things in
this case.

> 	Besides, as I’ve already noted, it does make tar-untar-buffer
> 	more consistent with both the rest of tar-mode.el, /and/ the
> 	rest of Emacs.  I thus believe that a single extra LoC is fully
> 	justified here.

Once again, we disagree, there's no doubt about that.  There's no need
to reiterate things you already wrote, as they have been duly noted.

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

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 14 Feb 2015 19:12:04 +0000

 >>>> And I’d still like to hear why you think that tar-untar-buffer
 >>>> /must/ use the value of default-directory local to a buffer
 >>>> /other/ than the one the user called this command from.

 >>> Because it's simpler, and makes the code easier to follow.

 >> My former patch expands the code by a single line; my latter one
 >> adds two more for a comment (while removing one level of nesting
 >> from a fair chunk of the code at the same time.)  How is that making
 >> it any harder to follow?

 > Length of code is only one factor, insignificant in this case.
 > Subtle dependencies between different parts of the code, as well as
 > overriding the default-directory value, is what complicates things in
 > this case.

	My second patch [1] doesn’t change the value of
	default-directory (instead, it uses expand-file-name explicitly
	at the write-region time.)  As for the subtle dependencies,
	could you point at anything specific?  TIA.

[1] http://debbugs.gnu.org/19865#msg44

[…]

-- 
FSF associate member #7257  np. Povus esti simple — Persone  … B6A0 230E 334A