all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#14925: 24.3.50; `image-dired.el' code (minor)
@ 2013-07-21 19:37 Drew Adams
  2013-07-21 19:54 ` Drew Adams
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Drew Adams @ 2013-07-21 19:37 UTC (permalink / raw)
  To: 14925

Generally, it would be better if `image-dired.el' followed the
conventions used in dired.el, dired-aux.el, and dired-x.el wrt (a)
naming and (b) parameter ARG (prefix argument).  That helps users by
providing relatively consistent command names and UI behavior.

Also a minor coding remark (#1).

1. `image-dired-dired-file-marked-p' uses regexp "^ .*$", which I think
is the same as "^ " (which is simpler).  I also wonder why it does not
just use `dired-re-mark'.

2. Commands that act on the marked files should perhaps follow the
naming convention used in dired.el, dired-aux.el, and dired-x.el:
`*-do-*'.  Thus perhaps rename `image-dired-dired-comment-files',
`image-dired-tag-files', `image-dired-delete-tag',
`image-dired-display-thumbs', `image-dired-copy-with-exif-file-name',
`image-dired-dired-edit-comment-and-tags', and
`image-dired-create-thumbs'.

The equivalent Dired command that acts on the current file only, when
available, is named similarly, but without the `*-do-*' part.

3. Those commands do not treat parameter ARG the same way as do the
Dired `*-do-*' commands.  For the image-dired commands, ARG is
essentially boolean (and should thus have a name that reflects that).
Wouldn't it be better for it to be the `prefix-numeric-value' and let
you act on the next ARG files?  That would give users more flexibility
and provide a more consistent UI.  Is there a downside I am missing?


In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-07-14 on ODIEONE
Bzr revision: 113423 lekktu@gmail.com-20130715004922-i67tg2ois14h3fpm
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 CFLAGS='-O0 -g3' CPPFLAGS='-Ic:/Devel/emacs/include'
 LDFLAGS='-Lc:/Devel/emacs/lib''





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

* bug#14925: 24.3.50; `image-dired.el' code (minor)
  2013-07-21 19:37 bug#14925: 24.3.50; `image-dired.el' code (minor) Drew Adams
@ 2013-07-21 19:54 ` Drew Adams
  2013-07-25 19:13 ` Stefan Monnier
  2021-09-06 10:12 ` Lars Ingebrigtsen
  2 siblings, 0 replies; 7+ messages in thread
From: Drew Adams @ 2013-07-21 19:54 UTC (permalink / raw)
  To: 14925

Forgot to mention: "selected" should be "marked" in the doc string
of `image-dired-delete-tag'.





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

* bug#14925: 24.3.50; `image-dired.el' code (minor)
  2013-07-21 19:37 bug#14925: 24.3.50; `image-dired.el' code (minor) Drew Adams
  2013-07-21 19:54 ` Drew Adams
@ 2013-07-25 19:13 ` Stefan Monnier
  2013-07-26  1:02   ` Drew Adams
  2021-09-06 10:12 ` Lars Ingebrigtsen
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2013-07-25 19:13 UTC (permalink / raw)
  To: Drew Adams; +Cc: 14925

> 1. `image-dired-dired-file-marked-p' uses regexp "^ .*$", which I think
> is the same as "^ " (which is simpler).

It's similar, except for (match-end 0) and the position of point after
re-search-forward.  I haven't looked at the code to see if the
difference matters.

> I also wonder why it does not just use `dired-re-mark'.

No idea.

> 2. Commands that act on the marked files should perhaps follow the
> naming convention used in dired.el, dired-aux.el, and dired-x.el:
> `*-do-*'.  Thus perhaps rename `image-dired-dired-comment-files',
> `image-dired-tag-files', `image-dired-delete-tag',
> `image-dired-display-thumbs', `image-dired-copy-with-exif-file-name',
> `image-dired-dired-edit-comment-and-tags', and
> `image-dired-create-thumbs'.

> The equivalent Dired command that acts on the current file only, when
> available, is named similarly, but without the `*-do-*' part.

> 3. Those commands do not treat parameter ARG the same way as do the
> Dired `*-do-*' commands.  For the image-dired commands, ARG is
> essentially boolean (and should thus have a name that reflects that).
> Wouldn't it be better for it to be the `prefix-numeric-value' and let
> you act on the next ARG files?  That would give users more flexibility
> and provide a more consistent UI.  Is there a downside I am missing?

2 and 3 sound like good ideas.


        Stefan





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

* bug#14925: 24.3.50; `image-dired.el' code (minor)
  2013-07-25 19:13 ` Stefan Monnier
@ 2013-07-26  1:02   ` Drew Adams
  2013-07-26  5:31     ` Drew Adams
  0 siblings, 1 reply; 7+ messages in thread
From: Drew Adams @ 2013-07-26  1:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 14925

> > 1. `image-dired-dired-file-marked-p' uses regexp "^ .*$", which I think
> > is the same as "^ " (which is simpler).
> 
> It's similar, except for (match-end 0) and the position of point after
> re-search-forward.  I haven't looked at the code to see if the
> difference matters.

It does not; the match data and the position of point are not used.
(The `-p' suffix provides a clue.)  There is currently only one occurrence
of `image-dired-dired-file-marked-p':

(if (image-dired-dired-file-marked-p)
    (dired-unmark 1)
  (dired-mark 1))





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

* bug#14925: 24.3.50; `image-dired.el' code (minor)
  2013-07-26  1:02   ` Drew Adams
@ 2013-07-26  5:31     ` Drew Adams
  0 siblings, 0 replies; 7+ messages in thread
From: Drew Adams @ 2013-07-26  5:31 UTC (permalink / raw)
  To: Drew Adams, Stefan Monnier; +Cc: 14925

> > > 1. `image-dired-dired-file-marked-p' uses regexp "^ .*$", which I think
> > > is the same as "^ " (which is simpler).
> >
> > It's similar, except for (match-end 0) and the position of point after
> > re-search-forward.  I haven't looked at the code to see if the
> > difference matters.
> 
> It does not; the match data and the position of point are not used.
> (The `-p' suffix provides a clue.)  There is currently only one occurrence
> of `image-dired-dired-file-marked-p':
> 
> (if (image-dired-dired-file-marked-p)
>     (dired-unmark 1)
>   (dired-mark 1))

It also uses `save-excursion' and `looking-at', not `re-search-forward'.

However, as it is in principle just a predicate it should probably also
use `looking-at-p', not `looking-at'.





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

* bug#14925: 24.3.50; `image-dired.el' code (minor)
  2013-07-21 19:37 bug#14925: 24.3.50; `image-dired.el' code (minor) Drew Adams
  2013-07-21 19:54 ` Drew Adams
  2013-07-25 19:13 ` Stefan Monnier
@ 2021-09-06 10:12 ` Lars Ingebrigtsen
  2021-09-06 21:49   ` bug#14925: [External] : " Drew Adams
  2 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-06 10:12 UTC (permalink / raw)
  To: Drew Adams; +Cc: 14925

Drew Adams <drew.adams@oracle.com> writes:

> 1. `image-dired-dired-file-marked-p' uses regexp "^ .*$", which I think
> is the same as "^ " (which is simpler).  I also wonder why it does not
> just use `dired-re-mark'.

I've now changed this function to use `dired-re-mark' instead in Emacs
28.

As for the other two points -- changing the names of all these dired
commands -- I don't think the gain we get from more regular function
names in this area outweighs the pain we'd inflict on users that may be
using the old names.  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] 7+ messages in thread

* bug#14925: [External] : Re: bug#14925: 24.3.50; `image-dired.el' code (minor)
  2021-09-06 10:12 ` Lars Ingebrigtsen
@ 2021-09-06 21:49   ` Drew Adams
  0 siblings, 0 replies; 7+ messages in thread
From: Drew Adams @ 2021-09-06 21:49 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 14925@debbugs.gnu.org

> As for the other two points -- changing the names of all these dired
> commands -- I don't think the gain we get from more regular function
> names in this area outweighs the pain we'd inflict on users that may be
> using the old names.  So I'm closing this bug report.

Renaming and deprecation of the old names is the
right approach, IMO.  There's no reason that
something that's deprecated can't continue to be
supported forever, if you like.

Dired has very consistent command and argument
behaviors, especially when it comes to commands
that act on or are affected by marks.  It's a
pity to keep this wart - in the name of what
exactly?

It's trivial to make the renaming 100% painless
to all users of the old names - they'd just
continue to work, as long as you like. 

And what about changing the behavior of the ARG
argument?  Or if you don't change its behavior
to fit Dired, what about changing its name to
reflect what it does?





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

end of thread, other threads:[~2021-09-06 21:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-21 19:37 bug#14925: 24.3.50; `image-dired.el' code (minor) Drew Adams
2013-07-21 19:54 ` Drew Adams
2013-07-25 19:13 ` Stefan Monnier
2013-07-26  1:02   ` Drew Adams
2013-07-26  5:31     ` Drew Adams
2021-09-06 10:12 ` Lars Ingebrigtsen
2021-09-06 21:49   ` bug#14925: [External] : " Drew Adams

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.