unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
@ 2018-03-25 16:36 Drew Adams
  2018-03-25 16:45 ` Drew Adams
  2018-03-25 16:50 ` Eli Zaretskii
  0 siblings, 2 replies; 21+ messages in thread
From: Drew Adams @ 2018-03-25 16:36 UTC (permalink / raw)
  To: 30938

Emacs 27 has introduced an unfortunate incompatible change.

It has added an extra optional arg, ERROR, to `dired-get-marked-files',
which is fine.

The bug is that calls to `dired-get-marked-files' have been changed all
over the place to systematically pass a non-nil value for optional arg
ERROR.  In general, this is completely inappropriate when the command
invoking `dired-get-marked-files' is not called interactively.

There is NO reason to suppose that a `user-error' occurred when a given
command is invoked non-interactively.

The logic behind this change is wrong - much too simplistic.  A non-nil
ERROR arg should perhaps be passed when called from some commands, but
typically only when such a command is called interactively.  Only then
might Emacs legitimately assume (and even this is arguable, in general)
that the user has committed an error.

Please revert this change as soon as possible, while you look for a
better way to do what you intended to do for it.  Please do not impose
raising such a user error on commands when they are not invoked
interactively.  It is perfectly reasonable for some such commands to be
invoked when there are no marked files, in which case the action should
typically be a no-op - it should not be to raise an error (much less a
"user" error).

In GNU Emacs 27.0.50 (build 3, x86_64-w64-mingw32)
 of 2018-03-21
Repository revision: e70d0c9e66d7a8609450b2889869d16aeb0363b5
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install -C 'CFLAGS=-O2 -static -g3''





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-25 16:36 bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files Drew Adams
@ 2018-03-25 16:45 ` Drew Adams
  2018-03-28 20:27   ` Juri Linkov
  2018-03-25 16:50 ` Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-25 16:45 UTC (permalink / raw)
  To: 30938

For example, `dired-do-isearch' could be called non-interactively
in a context where it should do nothing (or something else should
be done) if there are no marked files.  Something like this is
better:

(defun dired-do-isearch (&optional interactivep)
  "Search for a string through all marked files using Isearch.
When invoked interactively, raise an error if no files are marked."
  (interactive "p")
  (multi-isearch-files
   (dired-get-marked-files
     nil nil 'dired-nondirectory-p nil interactivep)))





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-25 16:36 bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files Drew Adams
  2018-03-25 16:45 ` Drew Adams
@ 2018-03-25 16:50 ` Eli Zaretskii
  1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-25 16:50 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938

> Date: Sun, 25 Mar 2018 09:36:08 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> 
> The bug is that calls to `dired-get-marked-files' have been changed all
> over the place to systematically pass a non-nil value for optional arg
> ERROR.  In general, this is completely inappropriate when the command
> invoking `dired-get-marked-files' is not called interactively.
> 
> There is NO reason to suppose that a `user-error' occurred when a given
> command is invoked non-interactively.
> 
> The logic behind this change is wrong - much too simplistic.  A non-nil
> ERROR arg should perhaps be passed when called from some commands, but
> typically only when such a command is called interactively.  Only then
> might Emacs legitimately assume (and even this is arguable, in general)
> that the user has committed an error.

Please provide at least one example (preferably more than one) of a
real-life use case where these changes get in the way.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-25 16:45 ` Drew Adams
@ 2018-03-28 20:27   ` Juri Linkov
  2018-03-28 23:45     ` Drew Adams
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2018-03-28 20:27 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938

> For example, `dired-do-isearch' could be called non-interactively
> in a context where it should do nothing (or something else should
> be done) if there are no marked files.  Something like this is
> better:
>
> (defun dired-do-isearch (&optional interactivep)
>   "Search for a string through all marked files using Isearch.
> When invoked interactively, raise an error if no files are marked."
>   (interactive "p")
>   (multi-isearch-files
>    (dired-get-marked-files
>      nil nil 'dired-nondirectory-p nil interactivep)))

Is there a better way than doing the same by adding such a formal arg
to all 12 affected commands?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-28 20:27   ` Juri Linkov
@ 2018-03-28 23:45     ` Drew Adams
  2018-03-29 20:04       ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-28 23:45 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 30938

> > For example, `dired-do-isearch' could be called non-interactively
> > in a context where it should do nothing (or something else should
> > be done) if there are no marked files.  Something like this is
> > better:
> >
> > (defun dired-do-isearch (&optional interactivep)
> >   "Search for a string through all marked files using Isearch.
> > When invoked interactively, raise an error if no files are marked."
> >   (interactive "p")
> >   (multi-isearch-files
> >    (dired-get-marked-files
> >      nil nil 'dired-nondirectory-p nil interactivep)))
> 
> Is there a better way than doing the same by adding such a formal arg
> to all 12 affected commands?

Are you asking whether adding an optional INTERACTIVEP arg to
commands that call `dired-get-marked-files', and passing that
as the 5th arg, is better than having them systematically raise
a `user-error' (i.e., even in the non-interactive case) when
`dired-get-marked-files' finds no marked files?

If so, yes; that was the point I was making in that mail.

There is not necessarily any user error - or any error at
all - when no files are marked (e.g., when the function
is called on a non-file-line part of the buffer).

That's what I've done in my code (dired+.el), for example:

;; Updated for Emacs 27-pretest-2 change in dired-get-marked-files signature.
;;  dired-get-marked-files: Added optional arg ERROR-IF-NONE-P.
;;  diredp-list-marked, diredp-insert-subdirs, dired-do-(i)search(-regexp),
;;  dired-do-query-replace-regexp, dired-do-find-marked-files,
;;  diredp-describe-marked-autofiles:
;;    Added optional arg INTERACTIVEP.
;;    Pass non-nil ERROR-IF-NONE-P to dired-get-marked-files when INTERACTIVEP.
;;    (See Emacs bug #30938.)

Emacs has already updated those 13 commands (there's one
also in dired-x.el) to add the 5th arg to their calls to
`dired-get-marked-files'.  The only further change needed
is to pass that arg as non-nil only when the command is
called interactively.  It is only in the interactive case
that we can know (assume) that such an error should be
raised.

[Dunno whether any of the 15 _other_ commands that call
`dired-get-marked-files' need similar treatment, i.e.,
those where NO 5th arg is passed yet (in dired.el,
image-dired.el, epa-dired.el, thumbs.el, and message.el).]






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-28 23:45     ` Drew Adams
@ 2018-03-29 20:04       ` Juri Linkov
  2018-03-29 20:25         ` Noam Postavsky
                           ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Juri Linkov @ 2018-03-29 20:04 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938

> Emacs has already updated those 13 commands (there's one
> also in dired-x.el) to add the 5th arg to their calls to
> `dired-get-marked-files'.  The only further change needed
> is to pass that arg as non-nil only when the command is
> called interactively.  It is only in the interactive case
> that we can know (assume) that such an error should be
> raised.

Don't you see there is something wrong in adding the same INTERACTIVEP
arg to all these 13 commands and possibly to more 15 other commands?
What I'm asking for is an alternative, e.g. to detect if the command is
called interactively and raise an error only in this case.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-29 20:04       ` Juri Linkov
@ 2018-03-29 20:25         ` Noam Postavsky
  2018-03-30  4:01           ` Drew Adams
  2018-03-30  4:01         ` Drew Adams
                           ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Noam Postavsky @ 2018-03-29 20:25 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 30938

On 29 March 2018 at 16:04, Juri Linkov <juri@linkov.net> wrote:

> Don't you see there is something wrong in adding the same INTERACTIVEP
> arg to all these 13 commands and possibly to more 15 other commands?

The parameter should be named THROW-ERROR-P, then it's clear that
there is nothing wrong with it, all commands just happen to share this
interface feature: they may throw error, or not.
(and they can still work correctly if called non-interactively from
some other interactive command)





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-29 20:04       ` Juri Linkov
  2018-03-29 20:25         ` Noam Postavsky
@ 2018-03-30  4:01         ` Drew Adams
  2018-03-30  7:57           ` Eli Zaretskii
  2018-04-02 19:30           ` Juri Linkov
       [not found]         ` <<8111e8b0-a7fb-4de4-9371-fd69c74c46e5@default>
                           ` (3 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Drew Adams @ 2018-03-30  4:01 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 30938

> > Emacs has already updated those 13 commands (there's one
> > also in dired-x.el) to add the 5th arg to their calls to
> > `dired-get-marked-files'.  The only further change needed
> > is to pass that arg as non-nil only when the command is
> > called interactively.  It is only in the interactive case
> > that we can know (assume) that such an error should be
> > raised.
> 
> Don't you see there is something wrong in adding the same INTERACTIVEP
> arg to all these 13 commands and possibly to more 15 other commands?
> What I'm asking for is an alternative, e.g. to detect if the command is
> called interactively and raise an error only in this case.

Instead of asking me if I don't see there is something
wrong with that, why don't you tell us what you think
is wrong with it?

I said from the beginning:

  Please revert this change as soon as possible,
  while you look for a better way to do what you
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  intended to do for it.

I'm open to other ways to do what is needed, if they
are better.  Feel free to propose something.

I'm fine with what I proposed - either proposal:

1. What I proposed at the outset: revert the bad change
   and do nothing until a better approach is decided on.

2. What I proposed in my follow-up: provide an INTERACTIVEP
   arg to distinguish interactive use, and (at most) raise
   a `user-error' only in the interactive-call case.

You asked if there was a better approach than doing #2.
I replied that #2 seems fine, to me.  But please feel free
to propose another approach, explaining why you think it's
better.

Someone apparently thought it was OK to change 13 commands
to ALWAYS raise an error in the no-files case.  Why are
you shocked to hear that I would be OK with changing those
same commands to not raise the error in the non-interactive
case - IOW, to return them to their longstanding behavior
in that case?

As for the other 15 commands: I don't know whether whoever
changed the 13 also considered the 15 and decided no error
was ever needed in their case.  But if not then the same
attention would be needed for them either to fix them as I
suggested (#2) or to break them as has been done for the
13.

I see no problem at all with having different behavior
when interactive and when not, especially when the only
difference is whether to raise an error in a corner case.

And the way to do that (explicitly recommended in the
manual) to make that distinction is to add an optional
INTERACTIVEP arg.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-29 20:25         ` Noam Postavsky
@ 2018-03-30  4:01           ` Drew Adams
  0 siblings, 0 replies; 21+ messages in thread
From: Drew Adams @ 2018-03-30  4:01 UTC (permalink / raw)
  To: Noam Postavsky, Juri Linkov; +Cc: 30938

> > Don't you see there is something wrong in adding the same INTERACTIVEP
> > arg to all these 13 commands and possibly to more 15 other commands?
> 
> The parameter should be named THROW-ERROR-P, then it's clear that
> there is nothing wrong with it, all commands just happen to share this
> interface feature: they may throw error, or not.
> (and they can still work correctly if called non-interactively from
> some other interactive command)

Actually, as you can see from the change log I cited,
I call the optional arg for `dired-get-marked-files'
ERROR-IF-NONE-P (which is better than THROW-ERROR-P,
which doesn't say what the error is about).

That's precisely what that optional arg means for
`dired-get-marked-files'.  And it will only ever
mean that, no doubt.

However, for the individual commands that call that
function, the arg can be called either that or just
INTERACTIVEP.

Yes, it is the case _currently_ that in those commands
distinguishing the interactive case is used only in the
call to `dired-get-marked-files', i.e., only to make it
raise the corner-case error, so far.  So I'm OK with
calling the arg ERROR-IF-NONE-P there also.  I'm also
OK with calling it INTERACTIVEP there.

In neither place should it be called THROW-ERROR-P, IMO.
But as long as the problem gets fixed I don't really care
much about the parameter name.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-30  4:01         ` Drew Adams
@ 2018-03-30  7:57           ` Eli Zaretskii
  2018-04-02 19:30           ` Juri Linkov
  1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-30  7:57 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938, juri

> Date: Thu, 29 Mar 2018 21:01:44 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 30938@debbugs.gnu.org
> 
> Instead of asking me if I don't see there is something
> wrong with that, why don't you tell us what you think
> is wrong with it?
> 
> I said from the beginning:
> 
>   Please revert this change as soon as possible,
>   while you look for a better way to do what you
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   intended to do for it.
> 
> I'm open to other ways to do what is needed, if they
> are better.  Feel free to propose something.
> 
> I'm fine with what I proposed - either proposal:
> 
> 1. What I proposed at the outset: revert the bad change
>    and do nothing until a better approach is decided on.
> 
> 2. What I proposed in my follow-up: provide an INTERACTIVEP
>    arg to distinguish interactive use, and (at most) raise
>    a `user-error' only in the interactive-call case.
> 
> You asked if there was a better approach than doing #2.
> I replied that #2 seems fine, to me.  But please feel free
> to propose another approach, explaining why you think it's
> better.
> 
> Someone apparently thought it was OK to change 13 commands
> to ALWAYS raise an error in the no-files case.  Why are
> you shocked to hear that I would be OK with changing those
> same commands to not raise the error in the non-interactive
> case - IOW, to return them to their longstanding behavior
> in that case?

A lot of discussion gone under the bridge, but I asked a question in
the very beginning that was apparently ignored:

  Please provide at least one example (preferably more than one) of a
  real-life use case where these changes get in the way.

Without an answer to that, I cannot see why we have to do anything
about this issue, because up front I see no problem here at all, not
one that has been spelled out.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
       [not found]           ` <<83lgeac7xs.fsf@gnu.org>
@ 2018-03-30 15:01             ` Drew Adams
  2018-03-30 15:30               ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-30 15:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 30938, juri

> A lot of discussion gone under the bridge, but I asked a question in
> the very beginning that was apparently ignored:
> 
>   Please provide at least one example (preferably more than one) of a
>   real-life use case where these changes get in the way.
> 
> Without an answer to that, I cannot see why we have to do anything
> about this issue, because up front I see no problem here at all, not
> one that has been spelled out.

I answered your question, pointing to the example of calling
`dired-do-isearch' (or any of the other such commands)
non-interactively - e.g., by creating a command that uses it.

If you don't believe that a user or another library could
or would define such a command (or in any other way would
call an existing function that calls the workhorse function
`dired-get-marked-files', then too bad.

In that case your idea of real-life use of Emacs Lisp differs
from mine; you will close the bug; and we will move on.  I've
already fixed my code to work around this change (except for
`dired-do-chxxx' repercussions - see below).

---

`dired-get-marked-files' is no less a utility function than
is, say, `dired-get-filename'.  And functions, including
commands, that call it are themselves candidates for reuse
in defining other functions, including other commands.

In addition, users often look to the definitions of Emacs
commands when creating similar or derivative commands.

Imagine that a user wants a command similar to
`dired-do-search' but that does things a bit differently.
S?he might look at the def of `dired-do-isearch' for
inspiration, and thus copy its use of `dired-marked-files'.
Hard-coding a non-nil 5th arg for it is a bad model.

(Same thing for `dired-do-search' etc. and the other
commands that, for some reason (?), have not yet
undergone this change.)

---

Similarly, for `dired-do-chxxx', which is NOT a command,
and which currently hardcodes a non-nil 5th arg to
`dired-get-marked-files' just like the commands do.

To fix that aberration a bit more surgery is required,
no doubt.  We could add an optional ERROR-IF-NONE-P arg
to `dired-do-chxxx', and obtain that from an optional
INTERACTIVEP arg to `dired-do-chmod' etc.  IOW, same
kind of fix.  Or perhaps Juri has a better fix in mind.

And as for "real-life" examples, BTW, I do define
`-mouse-' versions of the `dired-do-ch*' commands.
E.g.:

(defun diredp-mouse-do-chmod (event)
  "Change the mode of this file.
This calls chmod, so symbolic modes like `g+w' are allowed."
  (interactive "e")
  (let ((mouse-pos  (event-start event)))
    (select-window (posn-window mouse-pos))
    (goto-char (posn-point mouse-pos)))
  (dired-do-chxxx "Mode" "chmod" 'chmod 1)
  (diredp-previous-line 1))

As I haven't yet bothered to fix the now-bugged
`dired-do-chxxx' locally, such "real-life" commands
of mine are still broken by this recent change (for
the moment).

You can do nothing to fix this bug, and wait to see
whether Drew is the only one who has or will ever
have code that gets bitten by it.  Or you can DTRT
and not hardcode raising an error systematically
whenever `dired-get-marked-files' would come up
with no files.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-30 15:01             ` Drew Adams
@ 2018-03-30 15:30               ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-30 15:30 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938, juri

> Date: Fri, 30 Mar 2018 08:01:42 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: juri@linkov.net, 30938@debbugs.gnu.org
> 
> >   Please provide at least one example (preferably more than one) of a
> >   real-life use case where these changes get in the way.
> > 
> > Without an answer to that, I cannot see why we have to do anything
> > about this issue, because up front I see no problem here at all, not
> > one that has been spelled out.
> 
> I answered your question, pointing to the example of calling
> `dired-do-isearch' (or any of the other such commands)
> non-interactively - e.g., by creating a command that uses it.

I don't see how I was supposed to understand that this as an answer to
my question: my question was not cited, and the text you posted had no
immediate relation to the issue at hand, it just says that
dired-get-marked-files could be called non-interactively, and intended
to do nothing when no files were marked, which is already possible.  I
don't want to guess what that has to do with the issue you raised, and
why.  Please spell that out.

> If you don't believe that a user or another library could
> or would define such a command (or in any other way would
> call an existing function that calls the workhorse function
> `dired-get-marked-files', then too bad.

Such uses can always call dired-get-marked-files with the last
argument nil or omitted.  Where's the problem?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
       [not found]               ` <<83k1tt8ttp.fsf@gnu.org>
@ 2018-03-30 15:43                 ` Drew Adams
  2018-03-30 16:07                   ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-30 15:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 30938, juri

> > If you don't believe that a user or another library could
> > or would define such a command (or in any other way would
> > call an existing function that calls the workhorse function
> > `dired-get-marked-files', then too bad.
> 
> Such uses can always call dired-get-marked-files with the last
> argument nil or omitted.  Where's the problem?

I won't repeat myself this time.  If you don't want to see or
fix the problem, so be it.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-30 15:43                 ` Drew Adams
@ 2018-03-30 16:07                   ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-30 16:07 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938, juri

> Date: Fri, 30 Mar 2018 08:43:29 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: juri@linkov.net, 30938@debbugs.gnu.org
> 
> > > If you don't believe that a user or another library could
> > > or would define such a command (or in any other way would
> > > call an existing function that calls the workhorse function
> > > `dired-get-marked-files', then too bad.
> > 
> > Such uses can always call dired-get-marked-files with the last
> > argument nil or omitted.  Where's the problem?
> 
> I won't repeat myself this time.  If you don't want to see or
> fix the problem, so be it.

I might agree that it's a bug if I understand it.  But since you
refuse to explain it, I guess I never will.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
       [not found]                   ` <<83in9d8s4b.fsf@gnu.org>
@ 2018-03-30 17:12                     ` Drew Adams
  2018-03-31  9:07                       ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-30 17:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 30938, juri

> I might agree that it's a bug if I understand it.  But since you
> refuse to explain it, I guess I never will.

Maybe this will help (but I doubt it) -

1. Existing Dired+ command (updated to accommodate this bug by
adding INTERACTIVEP and passing that to `dired-get-marked-files'):

(defun diredp-insert-subdirs (&optional switches interactivep)
  "Insert the marked subdirectories.
Like using \\<dired-mode-map>`\\[dired-maybe-insert-subdir]' at \
each marked directory line."
  (interactive
   (list (and current-prefix-arg
              (read-string
               "Switches for listing: "
               (or dired-subdir-switches  dired-actual-switches)))
         t))
  (dolist (subdir
            (dired-get-marked-files
             nil nil
             (lambda (fl)
               (and (file-directory-p fl)
                    (not (diredp-string-match-p "/[.][.]?\\'" fl))))
             nil
             interactivep))
    (dired-maybe-insert-subdir subdir switches)))

Imaginary function that uses that command as a utility:

(defun insert-marked-subdirs-and-do-stuff ()
  (DO-STUFF)
  (diredp-insert-subdirs)
  (DO-OTHER-STUFF))

If you assume that the insertion of marked subdirs is
not a requirement for the main behavior of this function,
e.g., that it acts on any that get inserted but it doesn't
_require_ any such insertions to do its job in general,
then without some change as proposed this function can
end in error without ever trying to DO-OTHER-STUFF.

I'm arguing that similar considerations can apply to
other existing commands.

Whether each such case is actually problematic is not
so important (IMO).  The point is for individual such
commands to, a priori, treat the non-interactive case
separately.

A non-interactive use case for an arbitrary command that
calls ` dired-get-marked-files' does not necessarily
have `user-error' as the right behavior for an empty set
of marked files.  That's all I'm saying.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-30 17:12                     ` Drew Adams
@ 2018-03-31  9:07                       ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-31  9:07 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938, juri

> Date: Fri, 30 Mar 2018 10:12:13 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: juri@linkov.net, 30938@debbugs.gnu.org
> 
> A non-interactive use case for an arbitrary command that
> calls ` dired-get-marked-files' does not necessarily
> have `user-error' as the right behavior for an empty set
> of marked files.  That's all I'm saying.

I'm sorry, but you lost me.  Let me try explaining this from my POV.

First, dired-get-marked-files _can_ be invoked when no files are
marked, without risking a user-error: it has the ARG argument for
that.  So if your hypothetical command needs for some reason to work
when no files are marked, it can, at least in principle.  Why your
dired-insert-subdirs didn't go that way, or at least didn't pass nil
as ERROR, I don't understand (and I'm not sure it's related to the
issue at hand).

It is true that dired-aux.el commands that call dired-get-marked-files
all pass nil as ARG (and t as ERROR), but what would be the semantics
of, say, dired-do-isearch-regexp when no files are marked?  That
command, and all the others which call dired-get-marked-files in the
same manner, is to do something on "all marked files", as their doc
string clearly says.

So why do we need to allow these commands to be invoked when no files
are marked, and expect them not to signal a user-error?

IOW, even if dired-do-isearch-regexp and its ilk are invoked
non-interactively, they are invoked as result of some command up the
call-stack, so the invocation does come from the user.  And since
dired-do-isearch-regexp etc. are documented to act on marked files, it
sounds appropriate to signal an error when no files are marked.  A
feature which wants to allow users to get away silently in this case
can always check whether any files are marked and avoid calling
dired-do-isearch-regexp etc. if none are.

That is why I asked for real-time use-cases where this gets in the
way: your argument sounds like a general philosophical issue to me:
you claim that it's "completely inappropriate" for a utility function
to signal an error on theoretical grounds, while in reality I still
don't see any practical problem that cannot be easily resolved.  If
your diredp-insert-subdirs is an example of a problem, then as I said
above, I don't understand why you couldn't just omit the last optional
argument to dired-get-marked-files, and be done.  (And since ERROR was
_added_ in Emacs 27, your original code should have been already
future-proof against such changes anyway.)





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
       [not found]                       ` <<838ta88vfz.fsf@gnu.org>
@ 2018-03-31 16:10                         ` Drew Adams
  2018-03-31 16:50                           ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Drew Adams @ 2018-03-31 16:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 30938, juri

> > A non-interactive use case for an arbitrary command that
> > calls ` dired-get-marked-files' does not necessarily
> > have `user-error' as the right behavior for an empty set
> > of marked files.  That's all I'm saying.
> 
> I'm sorry, but you lost me.  Let me try explaining this from my POV.
> 
> First, dired-get-marked-files _can_ be invoked when no files are
> marked, without risking a user-error: it has the ARG argument for
> that.  So if your hypothetical command needs for some reason to work
> when no files are marked, it can, at least in principle.  Why your
> dired[p]-insert-subdirs didn't go that way, or at least didn't pass nil
> as ERROR, I don't understand (and I'm not sure it's related to the
> issue at hand).

Don't confuse a command that invokes `dired-get-marked-files'
with a command that invokes a command that invokes
`dired-get-marked-files'.  Their behaviors can differ (and
in the general case they do), including wrt interactive and
non-interactive use.

`diredp-insert-subdirs' inserts the subdir of the current
line, if no subdirs are marked.  If none are marked and it
is called from a non-subdir line then it _should_ raise an
error interactively (and it does).

Should it also do so non-interactively?  Why should it
decide that?  Why not let its caller do that?  When
called non-interactively it cannot (and need not) know
what is the right way to handle that case.

Imaginary command `insert-marked-subdirs-and-do-stuff'
should _not_ raise an error, and its invocation of
`diredp-insert-subdirs' should be a _no-op_, in the
same situation where invoking `diredp-insert-subdirs'
interactively would raise an error.

Code can tell `dired-get-marked-files' directly whether
it should raise an error if there are no files gathered.
What's missing is for callers of `dired-get-marked-files'
to tell it that conditionally, based on whether they are
called interactively.

Being able to do that is for the benefit of reusing a
command that calls `dired-get-marked-files' in different
ways.  It is not needed for the benefit of a command
that calls `dired-get-marked-files' directly, as that
choice is already under its control.

> It is true that dired-aux.el commands that call dired-get-marked-files
> all pass nil as ARG (and t as ERROR), but what would be the semantics
> of, say, dired-do-isearch-regexp when no files are marked?  That
> command, and all the others which call dired-get-marked-files in the
> same manner, is to do something on "all marked files", as their doc
> string clearly says.
> 
> So why do we need to allow these commands to be invoked when no files
> are marked, and expect them not to signal a user-error?

For the benefit of a function that might call them and not
want to raise that error.

The interactive case for a command that calls
`dired-get-marked-files' is completely known by that
command.  The context of a non-interactive use of the same
command is not known to it.  Its code should not prejudice
the behavior intended by its caller.

In the example of `insert-marked-subdirs-and-do-stuff', its
call to `diredp-insert-subdirs' is _not_ its main purpose,
and it is fine if there are no marked subdirs and thus none
get inserted.  For `i-m-s-a-d-s' to do its overall job, the
call to `d-i-s' needs to tolerate no subdirs being marked
and inserted, by being benign - a no-op.

Although no marked subdirs (and point not on a subdir) _is_
a user error for interactively called `d-i-s', it is _not_
an error for interactively (or not) `i-m-s-a-d-s'.

> IOW, even if dired-do-isearch-regexp and its ilk are invoked
> non-interactively, they are invoked as result of some command up the
> call-stack, so the invocation does come from the user. 

The invocation of `d-d-i-r' comes ultimately from the user,
as does the invocation of pretty much everything in Emacs.

But the intention and purpose of `d-d-i-r' can be different
- including wrt what no markings of particular files might
mean - from the intention and purpose of an upstream command
that directly or indirectly invokes `d-d-i-r'.

There is no reason to assume that interactive and
non-interactive use of a given command should have the same
behavior wrt the new `dired-get-marked-files' arg ERROR.
Such hardcoding provides no benefit and can get in the way.

> And since
> dired-do-isearch-regexp etc. are documented to act on marked files, it
> sounds appropriate to signal an error when no files are marked.

It is appropriate for _them_, i.e., when invoked interactively.
It is not necessarily appropriate for something that calls them.

> A feature which wants to allow users to get away silently in this case
> can always check whether any files are marked and avoid calling
> dired-do-isearch-regexp etc. if none are.

It shouldn't have to.  You are essentially suggesting that it
too should invoke `dired-get-marked-files'.  That shouldn't
be necessary (not to mention that it can be costly).

> That is why I asked for real-time use-cases where this gets in the
> way: your argument sounds like a general philosophical issue to me:

Call it what you like.

Your argument hasn't even been presented so far, AFAICT.
What are supporting reasons for a claim that we should
_not_ distinguish interactive from non-interactive use
for such commands and pass this along to `d-g-m-f'?

> you claim that it's "completely inappropriate" for a utility function
> to signal an error on theoretical grounds, while in reality I still
> don't see any practical problem that cannot be easily resolved.  If
> your diredp-insert-subdirs is an example of a problem, then as I said
> above, I don't understand why you couldn't just omit the last optional
> argument to dired-get-marked-files, and be done.  (And since ERROR was
> _added_ in Emacs 27, your original code should have been already
> future-proof against such changes anyway.)

See above.

The addition of optional arg ERROR was a _good_ thing.
No, no code could have taken care of this before, without
it.  Typically, point is on a file line, and when no files
are marked the command acts (purposefully) on the file of
the current line.  It is only rarely that point is in a
buffer position where the new error kicks in.

Another, more common, case where the error can kick in is
(as for the case of `diredp-insert-subdirs') when a FILTER
passed to `dired-get-marked-files' results in an empty
file list.

While it is typically a user error if nothing is marked
and point is not on any file line, it is not necessarily
an error when neither of those is the case - even for an
interactive case.  The individual command needs to decide
whether that is an error case.

Similarly, for a command B that _invokes_ such a command
A.  It is up to B to decide whether to raise an error.
If A systematically causes `d-g-m-f' to raise an error
in a context where an error makes sense for A, that can
conflict with what makes sense for B.

To me, the solution is simple: let a command that invokes
`d-g-m-f' pass non-nil ERROR when the command is called
interactively.

That just gives the command (and its author) more choice.
No reason to assume that its interactive use is the same
as its non-interactive use, including wrt its use of
`dired-get-marked-files'.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-31 16:10                         ` Drew Adams
@ 2018-03-31 16:50                           ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2018-03-31 16:50 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938, juri

> Date: Sat, 31 Mar 2018 09:10:41 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: juri@linkov.net, 30938@debbugs.gnu.org
> 
> > First, dired-get-marked-files _can_ be invoked when no files are
> > marked, without risking a user-error: it has the ARG argument for
> > that.  So if your hypothetical command needs for some reason to work
> > when no files are marked, it can, at least in principle.  Why your
> > dired[p]-insert-subdirs didn't go that way, or at least didn't pass nil
> > as ERROR, I don't understand (and I'm not sure it's related to the
> > issue at hand).
> 
> Don't confuse a command that invokes `dired-get-marked-files'
> with a command that invokes a command that invokes
> `dired-get-marked-files'.  Their behaviors can differ (and
> in the general case they do), including wrt interactive and
> non-interactive use.

Summary: I'm not convinced.  Some details below.

> `diredp-insert-subdirs' inserts the subdir of the current
> line, if no subdirs are marked.  If none are marked and it
> is called from a non-subdir line then it _should_ raise an
> error interactively (and it does).
> 
> Should it also do so non-interactively?  Why should it
> decide that?  Why not let its caller do that?  When
> called non-interactively it cannot (and need not) know
> what is the right way to handle that case.

AFAIU, you have all the tools to make it do that, by judicious use of
the ARG and ERROR arguments.

> Imaginary command `insert-marked-subdirs-and-do-stuff'
> should _not_ raise an error, and its invocation of
> `diredp-insert-subdirs' should be a _no-op_, in the
> same situation where invoking `diredp-insert-subdirs'
> interactively would raise an error.

If diredp-insert-subdirs is designed to support that, its interface
should allow that.  Commands that aren't designed to support that have
no obligations to allow calling them like that.  IOW freedom of
applications to invoke commands and functions outside their intended
domain is not unlimited, and asking for it to be unlimited, or
insisting that the limits be made wider, no matter the costs, is IMO
unreasonable.

> Being able to do that is for the benefit of reusing a
> command that calls `dired-get-marked-files' in different
> ways.

Such reuses have their limitations.  One of them is when no files are
marked and there's no "current file".  Those commands signal an error
because they cannot fulfill their contract; silently doing nothing in
this case is against that contract.

> While it is typically a user error if nothing is marked
> and point is not on any file line, it is not necessarily
> an error when neither of those is the case - even for an
> interactive case.

Signaling an error is a widely accepted way of telling the caller "you
asked me to do something; I didn't, and here's why".  If worse comes
to worst, the caller can always catch the error and recover from it.

> To me, the solution is simple: let a command that invokes
> `d-g-m-f' pass non-nil ERROR when the command is called
> interactively.

That solution causes unreasonable complication on a dozen of commands
for no good reason, so I don't see why we should do that.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-03-30  4:01         ` Drew Adams
  2018-03-30  7:57           ` Eli Zaretskii
@ 2018-04-02 19:30           ` Juri Linkov
  2022-04-21 13:20             ` Lars Ingebrigtsen
  1 sibling, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2018-04-02 19:30 UTC (permalink / raw)
  To: Drew Adams; +Cc: 30938

> I'm open to other ways to do what is needed, if they
> are better.  Feel free to propose something.

I still don't see why raising the error should depend on command's
interactivity, i.e. why these commands should not signal a user-error
in the non-interactive case.

We have thousands calls of such core functions as
(search-forward STRING &optional BOUND NOERROR COUNT)
and none of them binds the NOERROR arg to the command INTERACTIVE arg.
That's because it would make the control flow unmanageable in
a chain of commands, i.e. when a command that invokes a command
that invokes such core functions.

In very rare cases when you absolutely need to ignore errors,
just use (ignore-errors (dired-do-chmod))





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2018-04-02 19:30           ` Juri Linkov
@ 2022-04-21 13:20             ` Lars Ingebrigtsen
  2022-04-21 15:01               ` Drew Adams
  0 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-21 13:20 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 30938

Skimming this bug report, it seems like the conclusion was that we don't
want to change anything here, so I'm closing this bug report.

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






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files
  2022-04-21 13:20             ` Lars Ingebrigtsen
@ 2022-04-21 15:01               ` Drew Adams
  0 siblings, 0 replies; 21+ messages in thread
From: Drew Adams @ 2022-04-21 15:01 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Juri Linkov; +Cc: 30938@debbugs.gnu.org

> Skimming this bug report, it seems like the conclusion was that we
> don't want to change anything here, so I'm closing this bug report.

Very unfortunate.

For the record, and for Someone who might care to
fix this mistake in the future, here's a summary,
with text from previous messages.
___

There's no reason to assume that interactive and
non-interactive use of a given command should have
the same behavior wrt `dired-get-marked-files' arg
ERROR. Such hardcoding provides no benefit and can
get in the way.

A non-interactive use case for an arbitrary command
that calls ` dired-get-marked-files' does not
necessarily have `user-error' as the right behavior
for an empty set of marked files.

A common use case where the error can kick in is
when a FILTER passed to `dired-get-marked-files'
results in an empty file list.

While it is typically a user error if nothing is
marked and point is not on any file line, it is
not necessarily an error when neither of those is
the case - even for an interactive case.  The
individual command needs to decide whether that
is an error case.

Similarly, for a command B that _invokes_ such a
command A.  It's up to _B to decide_ whether to
raise an error.  If A systematically causes
`dired-get-marked-files' to raise an error in a
context where an error makes sense for A, that
can conflict with what makes sense for B.





^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-04-21 15:01 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-25 16:36 bug#30938: 27.0; `dired-do-create-files' etc.: do NOT always raise error if no files Drew Adams
2018-03-25 16:45 ` Drew Adams
2018-03-28 20:27   ` Juri Linkov
2018-03-28 23:45     ` Drew Adams
2018-03-29 20:04       ` Juri Linkov
2018-03-29 20:25         ` Noam Postavsky
2018-03-30  4:01           ` Drew Adams
2018-03-30  4:01         ` Drew Adams
2018-03-30  7:57           ` Eli Zaretskii
2018-04-02 19:30           ` Juri Linkov
2022-04-21 13:20             ` Lars Ingebrigtsen
2022-04-21 15:01               ` Drew Adams
     [not found]         ` <<8111e8b0-a7fb-4de4-9371-fd69c74c46e5@default>
     [not found]           ` <<83lgeac7xs.fsf@gnu.org>
2018-03-30 15:01             ` Drew Adams
2018-03-30 15:30               ` Eli Zaretskii
     [not found]         ` <<<8111e8b0-a7fb-4de4-9371-fd69c74c46e5@default>
     [not found]           ` <<<83lgeac7xs.fsf@gnu.org>
     [not found]             ` <<ea1c9d9f-2405-4377-bd42-de7f020cf9d4@default>
     [not found]               ` <<83k1tt8ttp.fsf@gnu.org>
2018-03-30 15:43                 ` Drew Adams
2018-03-30 16:07                   ` Eli Zaretskii
     [not found]         ` <<<<8111e8b0-a7fb-4de4-9371-fd69c74c46e5@default>
     [not found]           ` <<<<83lgeac7xs.fsf@gnu.org>
     [not found]             ` <<<ea1c9d9f-2405-4377-bd42-de7f020cf9d4@default>
     [not found]               ` <<<83k1tt8ttp.fsf@gnu.org>
     [not found]                 ` <<ceb6e79f-5f03-45a5-a7a4-5fe954661d5d@default>
     [not found]                   ` <<83in9d8s4b.fsf@gnu.org>
2018-03-30 17:12                     ` Drew Adams
2018-03-31  9:07                       ` Eli Zaretskii
     [not found]         ` <<<<<8111e8b0-a7fb-4de4-9371-fd69c74c46e5@default>
     [not found]           ` <<<<<83lgeac7xs.fsf@gnu.org>
     [not found]             ` <<<<ea1c9d9f-2405-4377-bd42-de7f020cf9d4@default>
     [not found]               ` <<<<83k1tt8ttp.fsf@gnu.org>
     [not found]                 ` <<<ceb6e79f-5f03-45a5-a7a4-5fe954661d5d@default>
     [not found]                   ` <<<83in9d8s4b.fsf@gnu.org>
     [not found]                     ` <<9b80ae9e-06e3-4217-89b1-eb8a3b0c93b8@default>
     [not found]                       ` <<838ta88vfz.fsf@gnu.org>
2018-03-31 16:10                         ` Drew Adams
2018-03-31 16:50                           ` Eli Zaretskii
2018-03-25 16:50 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).