unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Drew Adams <drew.adams@oracle.com>
To: Tino Calancha <tino.calancha@gmail.com>, Juri Linkov <juri@linkov.net>
Cc: 30285@debbugs.gnu.org, jidanni@jidanni.org
Subject: bug#30285: dired-do-chmod vs. top line of dired
Date: Thu, 1 Feb 2018 08:10:51 -0800 (PST)	[thread overview]
Message-ID: <d9791669-7282-4e3f-8f31-73b464bb58b1@default> (raw)
In-Reply-To: <alpine.DEB.2.20.1802011701390.19997@calancha-pc>

> I would like all `dired-do...' commands behave the same under the
> 'X condition': * called from the top line ** no marked files.

I've already said that it's not only about the top line.
It's about the ordinary Dired situation of not being on a
file line.  Plenty of Dired code already deals (simply)
with this "X condition".

The important things about a command that acts on the
marked files are that (1) it acts on the N next files,
if given a numeric prefix arg, (2) if no prefix arg,
it acts on the marked files, if any, (3) if not prefix
arg and no marked files it acts on the file of the
current line, and (4) it doesn't do something confusing
if there is no prefix arg, there are no marked files,
and there is no file on the current line. 

This bug is only about case (4) - a corner case.  The
solution is to just let the user know that s?he is not
on a file line.

> >> No, we don't need a function `dired-marked-files-or-file-at-point-p',
> >> for that or anything else.  The `dired-do-*' commands already DTRT
> >> wrt the marked-files-or-file-at-point.
> >
> > I agree that it's better to check the ‘files’ returned from
> > ‘dired-get-marked-files’.
> 
> Today I took a deeper look in the train and I saw there are
> several more commands that don't protect against X.  Some
> even breaks (e.g., dired-do-shell-command,
> dired-do-async-shell-command).
> 
> Below patch introduce a macro to systematically handle the 'X
> condition', what do you think?

Sorry, Tino, but I don't have the time or the will to
check your large patch.

My impression is that you guys are going overboard.

This bug is a _trivial_ usability bug.  There is nothing
really wrong with the existing behavior.  But yes, it
is unnecessary, and it could be slightly confusing, to
prompt the user about acting on zero files.

The trivial bug should be fixed; sure.  But it's not
a big deal.

The fix is also trivial.  Dired already provides what
is needed, as has been pointed out: just check the
return value of `dired-get-marked-files'.

If no files are marked then handle use of the command
as a `user-error': inform the user that there is no
file on the current line and no files are marked.
End of story.

This is trivial to do, and the resulting (corner case)
behavior will be simple, consistent, and clear to all
users.

If some `dired-do-*' command does not already use
`dired-get-marked-files' up front, and if it is not
appropriate for it to use it, then there are at least
two other alternatives that Dired offers to detect
this corner case.  I already mentioned them.  Using
either of them to fix the bug is also trivial.

In sum: trivial problem, trivial solution.

I'm not happy seeing big changes made to Dired code
gratuitously.  I don't have the time to argue about
it or provide alternative patches.  But I hope you
will not go down the road you seem to be going down.
Just one opinion.





  parent reply	other threads:[~2018-02-01 16:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-29 12:32 bug#30285: dired-do-chmod vs. top line of dired 積丹尼 Dan Jacobson
2018-01-29 15:14 ` Tino Calancha
2018-01-29 16:05   ` Eli Zaretskii
2018-01-29 23:21     ` Tino Calancha
2018-01-29 23:42       ` Drew Adams
2018-01-30  3:53         ` Tino Calancha
2018-01-30  4:43           ` Drew Adams
2018-01-30 15:15             ` Drew Adams
2018-01-31  9:49               ` Tino Calancha
2018-01-31 19:04                 ` Drew Adams
2018-01-31 21:35         ` Juri Linkov
2018-01-31 23:20           ` Drew Adams
2018-02-01  8:16           ` Tino Calancha
2018-02-01  9:17             ` Tino Calancha
2018-02-01 16:10             ` Drew Adams [this message]
2018-02-04 23:12               ` Tino Calancha
2018-02-05 16:45                 ` Drew Adams
2018-02-01 20:07             ` Juri Linkov
2018-02-01 20:50               ` Drew Adams
2018-02-01 21:35                 ` Juri Linkov
2018-02-01 22:23                   ` Drew Adams
2018-02-03 22:23                     ` Juri Linkov
2018-02-04 10:02                       ` martin rudalics
2018-02-04 21:44                         ` Juri Linkov
2018-02-06 21:32                         ` Juri Linkov
2018-02-04 23:08                   ` Tino Calancha
2018-02-05 21:01                     ` Juri Linkov
2018-02-05 21:52                       ` Drew Adams
2018-01-29 15:24 ` 積丹尼 Dan Jacobson
2018-01-29 23:14   ` Tino Calancha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9791669-7282-4e3f-8f31-73b464bb58b1@default \
    --to=drew.adams@oracle.com \
    --cc=30285@debbugs.gnu.org \
    --cc=jidanni@jidanni.org \
    --cc=juri@linkov.net \
    --cc=tino.calancha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).