bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

emacs -Q ~ -eval "(require 'dired-x)"
C-u * . el RET
;; signal error:
Wrong type argument: char-or-string-p, (4)

Because the interactive specification read MARKER-CHAR using 'P'.
That would be sensical if prefix argument has the meaning of unmark
files.  That behaviour is found elsewhere in Dired, for instance,
`dired-mark-executables', `dired-mark-directories',
`dired-mark-files-regexp'.

In following patch, `dired-mark-extension' with a prefix argument
unmark files:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 8f24f389b19a700b6cf48c187273234ae889aae4 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Fri, 23 Sep 2016 23:06:28 +0900
Subject: [PATCH] dired-mark-extension: With prefix arg unmark files

* lisp/dired-x.el (dired-mark-extension): When called interactively
with prefix argument, then unmark files (Bug#24518).
---
  lisp/dired-x.el | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 41c2256..834c0de 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -331,10 +331,13 @@ dired-extra-startup
  ;; Mark files with some extension.
  (defun dired-mark-extension (extension &optional marker-char)
    "Mark all files with a certain EXTENSION for use in later commands.
+A prefix argument means to unmark them instead.
  A `.' is *not* automatically prepended to the string entered.
  EXTENSION may also be a list of extensions instead of a single one.
  Optional MARKER-CHAR is marker to use."
-  (interactive "sMarking extension: \nP")
+  (interactive
+   (list (read-string "Marking extension: ")
+	 (and current-prefix-arg ?\s)))
    (or (listp extension)
        (setq extension (list extension)))
    (dired-mark-files-regexp
-- 
2.9.3


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.2.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.0)
  of 2016-09-23
Repository revision: 65dc67c87553bec4c8b36432246b13f09d653118

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 23 Sep 2016 23:13:52 +0900 (JST)
> Cc: tino.calancha@gmail.com
> 
> 
> emacs -Q ~ -eval "(require 'dired-x)"
> C-u * . el RET
> ;; signal error:
> Wrong type argument: char-or-string-p, (4)
> 
> Because the interactive specification read MARKER-CHAR using 'P'.
> That would be sensical if prefix argument has the meaning of unmark
> files.  That behaviour is found elsewhere in Dired, for instance,
> `dired-mark-executables', `dired-mark-directories',
> `dired-mark-files-regexp'.
> 
> In following patch, `dired-mark-extension' with a prefix argument
> unmark files:

This loses the feature whereby the user could also specify the
character to use as the marker.  Try "C-u 65 * . el RET" to see what
that does.

While using the numeric code of a character sounds like a kludge, the
current implementation seems to do that deliberately.  It might make
sense to use 'c' instead of 'P' there, I think, but your patch totally
changes the semantics of the prefix argument, so I'm not sure I can
endorse such a change.

Thanks.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

On Sat, 24 Sep 2016, Eli Zaretskii wrote:

> This loses the feature whereby the user could also specify the
> character to use as the marker.  Try "C-u 65 * . el RET" to see what
> that does.
That feature has never being available since `dired-x' was added to Emacs
in 1994: the bug came from the very first commit.  No user will miss
that feature because nobody ever could use it.


> While using the numeric code of a character sounds like a kludge, the
> current implementation seems to do that deliberately.  It might make 
> sense to use 'c' instead of 'P' there, I think, but your patch totally
> changes the semantics of the prefix argument, so I'm not sure I can 
> endorse such a change.
Sure, we could use 'c' and fix it.  We could provide such feature not working
in >20 years without nobody oppening a bug report.  That sounds like a
not useful feature at all.
People often mark and unmark, but much less people mark with other
marks than ?*.  If they really want that, they still can call the
function non interactively, or use `dired-change-marks'.
Prompting user for the mark character is annoying: 99% of the times i just want
to be asked about the extension
Believe me, using a prefix argument for unmark is very useful.

IMO, my proposal makes a better command:
*) Interactively, this command will handle its optional argument as
    `dired-mark-files-regexp': a prefix argument sets marker-char ?\s, i.e.,
    unmark files.
*) In non interactive calls, the meaning of marker-char is, again, as
    `dired-mark-files-regexp': the character to use for mark the files.

For me this uniform behaviour between these two marking commands is
a very nice thing.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sat, 24 Sep 2016 21:30:11 +0900 (JST)
> cc: Tino Calancha <tino.calancha@gmail.com>, 24518@debbugs.gnu.org
> 
> 
> On Sat, 24 Sep 2016, Eli Zaretskii wrote:
> 
> > This loses the feature whereby the user could also specify the
> > character to use as the marker.  Try "C-u 65 * . el RET" to see what
> > that does.
> That feature has never being available since `dired-x' was added to Emacs
> in 1994: the bug came from the very first commit.  No user will miss
> that feature because nobody ever could use it.

??? The example I gave works with the unmodified sources.  When used
as I've shown, there's no bug, and the command does what I expect.  So
how can we be sure no one ever used it that way and won't miss this
optional behavior?

Can you explain your logic here?

> > While using the numeric code of a character sounds like a kludge, the
> > current implementation seems to do that deliberately.  It might make 
> > sense to use 'c' instead of 'P' there, I think, but your patch totally
> > changes the semantics of the prefix argument, so I'm not sure I can 
> > endorse such a change.
> Sure, we could use 'c' and fix it.  We could provide such feature not working
> in >20 years without nobody oppening a bug report.  That sounds like a
> not useful feature at all.

It's an existing feature.  I won't agree to removing features that
existed for the last 22 years without a very good reason.  We have no
good means of finding out whether anyone out there uses this; if
someone does, removing this feature will most probably be a source of
aggravation for them.

> Believe me, using a prefix argument for unmark is very useful.

I believe you.  But we will have to provide this feature without
breaking other existing features.

> For me this uniform behaviour between these two marking commands is
> a very nice thing.

I don't disagree.  I just don't want to break existing features.

Thanks.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

On Sat, 24 Sep 2016, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Sat, 24 Sep 2016 21:30:11 +0900 (JST)
>> cc: Tino Calancha <tino.calancha@gmail.com>, 24518@debbugs.gnu.org
>>
>>
>> On Sat, 24 Sep 2016, Eli Zaretskii wrote:
>>
>>> This loses the feature whereby the user could also specify the
>>> character to use as the marker.  Try "C-u 65 * . el RET" to see what
>>> that does.
>> That feature has never being available since `dired-x' was added to Emacs
>> in 1994: the bug came from the very first commit.  No user will miss
>> that feature because nobody ever could use it.
>
> ??? The example I gave works with the unmodified sources.  When used
> as I've shown, there's no bug, and the command does what I expect.  So
> how can we be sure no one ever used it that way and won't miss this
> optional behavior?
>
> Can you explain your logic here?
Maybe you are a genious.  Normal people when we are prompt for a 
character we expect we can input the textual representation of the 
character, for example: i will answer
K
instead of
75

You should admit that your example is tricky.  Compare how the user is
prompted with:
M-x dired-change-marks RET
;; This command expects 'textual' representation of the character.

I don't think my patch breaks any feature, because i consider
prompting fo a char in hexadecimal not a feature.  Maybe for robots,
but certainly not for human.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sat, 24 Sep 2016 22:02:01 +0900 (JST)
> cc: Tino Calancha <tino.calancha@gmail.com>, 24518@debbugs.gnu.org
> 
> >> That feature has never being available since `dired-x' was added to Emacs
> >> in 1994: the bug came from the very first commit.  No user will miss
> >> that feature because nobody ever could use it.
> >
> > ??? The example I gave works with the unmodified sources.  When used
> > as I've shown, there's no bug, and the command does what I expect.  So
> > how can we be sure no one ever used it that way and won't miss this
> > optional behavior?
> >
> > Can you explain your logic here?
> Maybe you are a genious.

I'm not a genius.  I just read the sources.

> Normal people when we are prompt for a 
> character we expect we can input the textual representation of the 
> character, for example: i will answer
> K
> instead of
> 75

There's no prompt, you must specify the marker via the prefix
argument.  With 'P', you must type the numeric codepoint of the
character; if we change it to 'c', users will be able to say "C-u x"
instead (where 'x' is the marker character they want).

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Andreas Schwab]

On Sep 24 2016, Eli Zaretskii <eliz@gnu.org> wrote:

>> > This loses the feature whereby the user could also specify the
>> > character to use as the marker.  Try "C-u 65 * . el RET" to see what
>> > that does.
>> That feature has never being available since `dired-x' was added to Emacs
>> in 1994: the bug came from the very first commit.  No user will miss
>> that feature because nobody ever could use it.
>
> ??? The example I gave works with the unmodified sources.  When used
> as I've shown, there's no bug, and the command does what I expect.  So
> how can we be sure no one ever used it that way and won't miss this
> optional behavior?

Does it actually make sense to have to specify the character by its
code?  That doesn't look like this was an intented feature (being
undocumented).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Tino Calancha <tino.calancha@gmail.com>,  24518@debbugs.gnu.org
> Date: Sat, 24 Sep 2016 15:15:01 +0200
> 
> Does it actually make sense to have to specify the character by its
> code?

Like I said: it looks a kludge, but it definitely isn't an accident.
I suggested to replace 'P' with 'c' in the interactive spec, to make
it less kludgey.

> That doesn't look like this was an intented feature (being
> undocumented).

It's documented, see the doc string.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

On Sat, 24 Sep 2016, Eli Zaretskii wrote:

> Like I said: it looks a kludge, but it definitely isn't an accident.
> I suggested to replace 'P' with 'c' in the interactive spec, to make
> it less kludgey.
I know i am an small ant here, but I must say that IMO, that 
solution is bad Emacs: i strongly disagree with it.
I hardly believe that original Sebastian Kremer intention was
to provide such behaviour.  I am sure he wanted the command
behaves as `dired-mark-files-regexp' does with respect the
optional arg.
Feel free to fix the bug as you wish and push it to the repository.
I don't want to do it myself, because i don't want my name appear in
the git log as the author of that solution.
I am sorry if these words sounds bad.  It's not my intention; it's
good having different opinions.
>> That doesn't look like this was an intented feature (being
>> undocumented).
>
> It's documented, see the doc string.
Andreas meant in his previous e-mail that if the feature requires
that the user input an hexadecimal, then that should appear in the
doc string.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sat, 24 Sep 2016 23:06:46 +0900 (JST)
> cc: Andreas Schwab <schwab@linux-m68k.org>, tino.calancha@gmail.com, 
>     24518@debbugs.gnu.org
> 
> Feel free to fix the bug as you wish and push it to the repository.
> I don't want to do it myself, because i don't want my name appear in
> the git log as the author of that solution.

Done.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Andreas Schwab]

On Sep 24 2016, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: Tino Calancha <tino.calancha@gmail.com>,  24518@debbugs.gnu.org
>> Date: Sat, 24 Sep 2016 15:15:01 +0200
>> 
>> Does it actually make sense to have to specify the character by its
>> code?
>
> Like I said: it looks a kludge, but it definitely isn't an accident.

What makes you that confident?  It look certainly like a
misunderstanding.

> I suggested to replace 'P' with 'c' in the interactive spec, to make
> it less kludgey.

"c" would make it non-optional, definitely not intented.

>> That doesn't look like this was an intented feature (being
>> undocumented).
>
> It's documented, see the doc string.

It isn't.  There is no occurence of "prefix" in the doc string.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: tino.calancha@gmail.com,  24518@debbugs.gnu.org
> Date: Sat, 24 Sep 2016 18:48:35 +0200
> 
> > Like I said: it looks a kludge, but it definitely isn't an accident.
> 
> What makes you that confident?  It look certainly like a
> misunderstanding.
> 
> > I suggested to replace 'P' with 'c' in the interactive spec, to make
> > it less kludgey.
> 
> "c" would make it non-optional, definitely not intented.
> 
> >> That doesn't look like this was an intented feature (being
> >> undocumented).
> >
> > It's documented, see the doc string.
> 
> It isn't.  There is no occurence of "prefix" in the doc string.

I believe my recent commit fixed all of the above.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

On Sat, 24 Sep 2016, Eli Zaretskii wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: tino.calancha@gmail.com,  24518@debbugs.gnu.org
>> Date: Sat, 24 Sep 2016 18:48:35 +0200
>>
>>> Like I said: it looks a kludge, but it definitely isn't an accident.
>>
>> What makes you that confident?  It look certainly like a
>> misunderstanding.
>>
>>> I suggested to replace 'P' with 'c' in the interactive spec, to make
>>> it less kludgey.
>>
>> "c" would make it non-optional, definitely not intented.
>>
>>>> That doesn't look like this was an intented feature (being
>>>> undocumented).
>>>
>>> It's documented, see the doc string.
>>
>> It isn't.  There is no occurence of "prefix" in the doc string.
>
> I believe my recent commit fixed all of the above
Yeah, you fixed something, and we are exactly in the same point
that before fixing such thing.  We didn't get something better.
The point here is that a Dired user would expect/wish that the
interactive call behaves differently.  I do.
Do you know how i noticed the bug?
Because i assumed that a prefix argument will unmark as other
Dired marks, and i got an error (!).
The right thing here is to unmark with a prefix argument: other thing than 
that would be unconsistent.  Not Dired style, and pretty useless.
For me its not a problem, because i use Dired+.  Other people are not as
lucky.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sun, 25 Sep 2016 02:14:14 +0900 (JST)
> cc: Andreas Schwab <schwab@linux-m68k.org>, tino.calancha@gmail.com, 
>     24518@debbugs.gnu.org
> 
> > I believe my recent commit fixed all of the above
> Yeah, you fixed something, and we are exactly in the same point
> that before fixing such thing.  We didn't get something better.

We don't have this command signal an error anymore, do we?  I think
that's a definite improvement.

Besides, you yourself told me to fix the bug, so I did.

> The point here is that a Dired user would expect/wish that the
> interactive call behaves differently.  I do.
> Do you know how i noticed the bug?
> Because i assumed that a prefix argument will unmark as other
> Dired marks, and i got an error (!).

But it isn't true that every Dired command that marks files will
unmark them if invoked with an argument.  E.g., dired-mark doesn't.
So this doesn't look like a universal convention in Dired, only a
frequent feature.

OTOH, since Dired allows the user to use different marker characters
in other places, IMO it is not very reasonable to exempt this
particular command.  Especially since it did have a (semi-broken) way
of doing that.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Tino Calancha]

On Sat, 24 Sep 2016, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Sun, 25 Sep 2016 02:14:14 +0900 (JST)
>> cc: Andreas Schwab <schwab@linux-m68k.org>, tino.calancha@gmail.com,
>>     24518@debbugs.gnu.org
>>
>>> I believe my recent commit fixed all of the above
>> Yeah, you fixed something, and we are exactly in the same point
>> that before fixing such thing.  We didn't get something better.
>
> We don't have this command signal an error anymore, do we?  I think
> that's a definite improvement.
Yes, it is.  But i doubt that in 22 years someone tried intentionally
such behaviour:  I guess you was the first in History when you proposed
your example.  Congratulations.

> Besides, you yourself told me to fix the bug, so I did.
Thank you very much.
>> The point here is that a Dired user would expect/wish that the
>> interactive call behaves differently.  I do.
>> Do you know how i noticed the bug?
>> Because i assumed that a prefix argument will unmark as other
>> Dired marks, and i got an error (!).
>
> But it isn't true that every Dired command that marks files will
> unmark them if invoked with an argument.  E.g., dired-mark doesn't.
> So this doesn't look like a universal convention in Dired, only a
> frequent feature.
The reason why dired-mark do not follow that idiom is because we have a 
dedicated command dired-unmark.  Indeed, you can see that it is very
universal convention that if we don't have the dedicated-unmark-command
for the task in hands, then the marking command _unmark_ with a prefix
argument.
> OTOH, since Dired allows the user to use different marker characters
> in other places, IMO it is not very reasonable to exempt this
> particular command.  Especially since it did have a (semi-broken) way
> of doing that.
There better ways to mark with a different character in Dired than that. 
We should respect the 'prefix arg unmark' idiom.

Regards

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Andreas Schwab]

On Sep 24 2016, Eli Zaretskii <eliz@gnu.org> wrote:

> OTOH, since Dired allows the user to use different marker characters
> in other places,

None of the dired commands do that.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Andreas Schwab]

On Sep 24 2016, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: tino.calancha@gmail.com,  24518@debbugs.gnu.org
>> Date: Sat, 24 Sep 2016 18:48:35 +0200
>> 
>> > Like I said: it looks a kludge, but it definitely isn't an accident.
>> 
>> What makes you that confident?  It look certainly like a
>> misunderstanding.
>> 
>> > I suggested to replace 'P' with 'c' in the interactive spec, to make
>> > it less kludgey.
>> 
>> "c" would make it non-optional, definitely not intented.
>> 
>> >> That doesn't look like this was an intented feature (being
>> >> undocumented).
>> >
>> > It's documented, see the doc string.
>> 
>> It isn't.  There is no occurence of "prefix" in the doc string.
>
> I believe my recent commit fixed all of the above.

No, it doesn't make sense to ask for the marker.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: tino.calancha@gmail.com,  24518@debbugs.gnu.org
> Date: Sat, 24 Sep 2016 21:13:01 +0200
> 
> > I believe my recent commit fixed all of the above.
> 
> No, it doesn't make sense to ask for the marker.

That's not a problem that needed fixing.  It's an optional feature, so
users who see no sense in it could simply not invoke the command with
an argument.

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[Andreas Schwab]

On Sep 24 2016, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: tino.calancha@gmail.com,  24518@debbugs.gnu.org
>> Date: Sat, 24 Sep 2016 21:13:01 +0200
>> 
>> > I believe my recent commit fixed all of the above.
>> 
>> No, it doesn't make sense to ask for the marker.
>
> That's not a problem that needed fixing.

Yes, it is.  It's a misfeature.  It makes it alien with all the other
dired commands.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#24518: 25.2.50; dired-mark-extension with prefix arg fails

[John Wiegley]

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

>>>>> "AS" == Andreas Schwab <schwab@linux-m68k.org> writes:

AS> Yes, it is. It's a misfeature. It makes it alien with all the other dired
AS> commands.

Hello, I'm just joining this conversation, but I've read all the backlog.

First, if Eli believes that maintenance requires retaining a 22 year old
feature -- however improbable its use, however idiosyncratic its behavior when
compared to other dired commands -- that's the end of the discussion here. I
ask that you respect this. You need to convince your maintainer of the merits
of the change, not argue that his decision is without ground. He has all the
ground he needs due to his role on this project.

Do I think it's a wart that should be fixed? Maybe. But we have to tread very
carefully here. We have a commitment to those whom we maintain Emacs for: to
avoid gratuitously changing long-standing behavior without considering every
angle. If we allowed everyone to change whatever annoyed them, just because
they thought it was an improvement, we'd be in a terrible state.

I'd like to see a couple of things happen from here:

 1. Begin a discussion on emacs-devel about the behavior of this command.
    Solicit the wisdom of other old-timers on that list. Present your case
    dispassionately, in terms of what we gain from such a change.

 2. Somehow poll our users to find out if, truly, no one uses this command. We
    may not find those people -- to date, we've been terrible at effective
    polling -- but if we can't find a single person who wants the feature, and
    many who want uniformity, at least we have some data to consider.

Calling each other reasonable or unreasonable, or debating this as though it
should "just be obvious" and thus Eli's opinion doesn't matter, is not the way
to make progress. _What he says goes_, because in the end, he controls who is
allowed to commit changes and who isn't.

Let's focus on the technical issues, and broaden the discussion. So far, a
heated debate between three people will never convince me that we need to
change 22 years of prior art, either.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]