bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Drew Adams]

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

`dired-hide-details-mode' is essentially a reimplementation of
dired-details (by Rob Giardina).  The Emacs 24 implementation of
hiding details is better (more efficient) that that of dired-details,
which is a good thing.

But the behavior is not as good for users as that available with
dired-details+.  Attached is a patch that rectifies this.  It gives
users more control.

These are the enhancements:

1. Users can decide whether the initial hide/show state of new Dired
   buffers reflects the last chosen state for a Dired buffer.  This is
   decided by option `dired-hide-details-propagate'.  Non-nil means
   propagate the last chosen state as the initial state of a new Dired
   buffer.

2. If `dired-hide-details-propagate' is nil, or if the user has not
   yet explicitly changed any Dired hide/show state, then option
   `dired-hide-details-initially' defines the initial state of a new
   Dired buffer.  IOW, it specifies what the "last" state defaults to.

In the patch the default value of each option is t, but this is open
for discussion.  In my experience with dired-details+ most users
prefer to hide details by default and to let the last chosen state
update the default state.

But if we wanted to keep the traditional behavior by default then we
would just set both options to nil by default.  The main point is to
give users a choice.

Note that regardless of the option values, once a user has chosen a
state for a given Dired buffer it remains in effect until s?he hits
`(' to choose a different state for _that buffer_.  IOW, the options
affect only new Dired buffers, that is, buffers where the user has not
yet used `('.  A user display choice for a given buffer is never
overridden.

Initialization behavior applies also to `C-x C-v RET', that is,
accepting the same directory as the alternate one to visit.  This is
in keeping with `find-alternate-file' resetting other Dired settings
(markings, omissions, display order, switches, etc.).  Reverting using
`g', however, keeps the user's chosen display state; i.e., unlike `C-x
C-v RET', the state is not reinitialized with `g'.

Reasons to have non-nil default values for both options:

a. `dired-hide-details-propagate': Hide details by default.
   Simpler, screen-space conservative.  Show details on demand: `('.

b. `dired-hide-details-propagate': Propagate last view by default.
   This essentially tells Emacs, "Don't do anything I haven't told you
   to do.  Just keep doing what I last told you to do until I tell you
   otherwise."

For more explanation of why it is a good idea for
`dired-hide-details-propagate' ti be non-nil by default, see the thread
on emacs-devel:
http://lists.gnu.org/archive/html/emacs-devel/2013-07/msg00644.html


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

[-- Attachment #2: dired-2013-07-20.patch --]
[-- Type: application/octet-stream, Size: 4142 bytes --]

diff -cw dired.el dired-patched-2013-07-20.el
*** dired.el	Sat Jul 20 12:10:57 2013
--- dired-patched-2013-07-20.el	Sat Jul 20 12:18:28 2013
***************
*** 249,254 ****
--- 249,266 ----
    :version "24.4"
    :group 'dired)
  
+ (defcustom dired-hide-details-initially t
+   "Non-nil means hide details in Dired from the outset."
+   :type 'boolean :version "24.4" :group 'dired)
+ 
+ (defcustom dired-hide-details-propagate t
+   "Non-nil means display the next Dired buffer the same way as the last.
+ The last `dired-hide-details-mode' value set is used by the next Dired
+ buffer created."
+   :type 'boolean :version "24.4" :group 'dired)
+ 
+ 
+ 
  ;; Internal variables
  
  (defvar dired-marker-char ?*		; the answer is 42
***************
*** 297,302 ****
--- 309,322 ----
  
  (put 'dired-actual-switches 'safe-local-variable 'dired-safe-switches-p)
  
+ (defvar dired-hide-details-last-state dired-hide-details-initially
+   "Last `dired-hide-details-mode' value.
+ Initialized to the value of option `dired-hide-details-initially'.")
+ 
+ (defvar dired-hide-details-toggled nil
+   "Non-nil means you have already toggled hiding details in this buffer.")
+ (make-variable-buffer-local 'dired-hide-details-toggled)
+ 
  (defvar dired-re-inode-size "[0-9 \t]*"
    "Regexp for optional initial inode and file size as made by `ls -i -s'.")
  
***************
*** 2265,2286 ****
        (substring file (match-end 0))
      file))
  \f
! ;;; Minor mode for hiding details
  ;;;###autoload
  (define-minor-mode dired-hide-details-mode
    "Hide details in Dired mode."
    :group 'dired
!   (unless (derived-mode-p 'dired-mode)
!     (error "Not a Dired buffer"))
    (dired-hide-details-update-invisibility-spec)
    (if dired-hide-details-mode
!       (add-hook 'wdired-mode-hook
! 		'dired-hide-details-update-invisibility-spec
! 		nil
! 		t)
!     (remove-hook 'wdired-mode-hook
! 		 'dired-hide-details-update-invisibility-spec
! 		 t)))
  
  (defun dired-hide-details-update-invisibility-spec ()
    (funcall (if dired-hide-details-mode
--- 2285,2309 ----
        (substring file (match-end 0))
      file))
  \f
! ;;; Minor modes for hiding details
! 
! ;;;###autoload
! (define-globalized-minor-mode global-dired-hide-details-mode dired-hide-details-mode
!   dired-hide-details-if-dired)
! 
  ;;;###autoload
  (define-minor-mode dired-hide-details-mode
      "Hide details in Dired mode."
+   (and dired-hide-details-propagate  dired-hide-details-last-state)
    :group 'dired
!   (unless (derived-mode-p 'dired-mode) (error "Not a Dired buffer"))
    (dired-hide-details-update-invisibility-spec)
+   (setq dired-hide-details-toggled  t)
+   (when dired-hide-details-propagate
+     (setq dired-hide-details-last-state  dired-hide-details-mode))
    (if dired-hide-details-mode
!       (add-hook 'wdired-mode-hook 'dired-hide-details-update-invisibility-spec nil t)
!     (remove-hook 'wdired-mode-hook 'dired-hide-details-update-invisibility-spec t)))
  
  (defun dired-hide-details-update-invisibility-spec ()
    (funcall (if dired-hide-details-mode
***************
*** 2299,2304 ****
--- 2322,2342 ----
  	     'remove-from-invisibility-spec)
  	   'dired-hide-details-link))
  
+ (defun dired-hide/show-details ()
+   "Hide/show details according to user options.
+ If `dired-hide-details-propagate' is non-nil and details have
+ never been hidden in the buffer, then hide/show according to your last
+ hide/show choice in any other Dired buffer or, if no last choice,
+ according to option `dired-hide-details-initially'."
+   (unless (or dired-hide-details-toggled ; No op if hide/show already set.
+               (buffer-narrowed-p)) ; No-op when showing just newly copied file etc.
+     (cond (dired-hide-details-propagate
+            (dired-hide-details-mode (if dired-hide-details-last-state 1 -1)))
+           (dired-hide-details-initially
+            (dired-hide-details-mode 1)))))
+ 
+ (add-hook 'dired-after-readin-hook #'dired-hide/show-details)
+ \f
  ;;; Functions for finding the file name in a dired buffer line.
  
  (defvar dired-permission-flags-regexp

Diff finished.  Sat Jul 20 12:20:35 2013

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Stefan Monnier]

> 1. Users can decide whether the initial hide/show state of new Dired
>    buffers reflects the last chosen state for a Dired buffer.  This is
>    decided by option `dired-hide-details-propagate'.  Non-nil means
>    propagate the last chosen state as the initial state of a new Dired
>    buffer.

> 2. If `dired-hide-details-propagate' is nil, or if the user has not
>    yet explicitly changed any Dired hide/show state, then option
>    `dired-hide-details-initially' defines the initial state of a new
>    Dired buffer.  IOW, it specifies what the "last" state defaults to.

Hmm... couldn't we merge those two configuration variables?
Here's the idea:
- Rename dired-hide-details-initially to dired-hide-details-default-mode,
  a new (global) minor mode which determines how new dired buffers show up.
- Make it so that toggling dired-hide-details-default-mode also toggles the
  dired-hide-details-mode in the current buffer.
This way, instead of setting dired-hide-details-propagate to t, users
can simply use dired-hide-details-default-mode instead of
dired-hide-details-mode.


        Stefan


PS: Your code often exceeds the 80 columns limit.

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Drew Adams]

> > 1. Users can decide whether the initial hide/show state of new Dired
> >    buffers reflects the last chosen state for a Dired buffer.  This is
> >    decided by option `dired-hide-details-propagate'.  Non-nil means
> >    propagate the last chosen state as the initial state of a new Dired
> >    buffer.
> 
> > 2. If `dired-hide-details-propagate' is nil, or if the user has not
> >    yet explicitly changed any Dired hide/show state, then option
> >    `dired-hide-details-initially' defines the initial state of a new
> >    Dired buffer.  IOW, it specifies what the "last" state defaults to.
> 
> Hmm... couldn't we merge those two configuration variables?

I supppose you mean shouldn't, not couldn't.

> Here's the idea:
> - Rename dired-hide-details-initially to dired-hide-details-default-mode,
>   a new (global) minor mode which determines how new dired buffers show up.
> - Make it so that toggling dired-hide-details-default-mode also toggles the
>   dired-hide-details-mode in the current buffer.
> This way, instead of setting dired-hide-details-propagate to t, users
> can simply use dired-hide-details-default-mode instead of
> dired-hide-details-mode.

I see no advantage, only disadvantage.  User options are generally the place
for default user settings.  Why have two minor modes here?  I might be
missing something, but to my mind that only confuses users and offers no
advantage.

We should stick to a single command and its key binding, and not make users
jump through hoops (yes, including changing key bindings) to choose the default
behavior they want (wrt initial hide/show and wrt propagation).

And if the same key and command are used (not two different mode commands),
a user can easily toggle the option value at any time and have the behavior
change - no fiddling with different commands or rebinding keys.

But if the *same* behavior is offered by your proposal as by mine, and if
users are not additionally bothered by your proposal, then I guess I
don't see a problem with it.  (That's an if.)

IMO:

1. The out-of-the-box default behavior should be as I described.

2. Users should be able to set their own preferred default behavior
persistently.  To me, that means a user option.  In any case, it
means not having to toggle anything interactively just to get the
preferred behavior as the saved one.

3. If the rest of the behavior I described is also provided, no problem.

In sum, it's the behavior I'm interested in.  If the behavior is intact
then I don't care much how you decide to implement it.

The code I sent is in Dired+ at least, so it is anyway available to users
who choose it, should you decide it is not the behavior you want for
vanilla Emacs.  Obviously I would prefer that (a) everyone benefit from
this feature and (b) I can then remove it from Dired+.  If you implement
something different then I will see whether I like its behavior enough
to remove what I have now in Dired+.

If you think I am missing something, please fill me in.

> Your code often exceeds the 80 columns limit.

Often?  Four of the 113 lines are longer than 80 chars (83, 84, 84, 85).

The longest line in dired.el is 103 chars!, both before and after patch.
The longest line in the patch I sent is 85 chars.  But feel free to
shorten any lines you like.

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Jambunathan K]

I want to KISS.   The following would meet my needs.

Suggest `dired-hide-details-mode' as one of the "available" options to
`dired-mode-hook'.  (May be through the :options keyword of defcustom.
Much like what is done for `emacs-lisp-mode-hook').

I have been using dired-hide-details-mode since it has been checked in
to repo.  I never felt the need for "propagate".  It is hit the toggle
instead.

I recommend `@' as the keybinding instead of `(' for toggling.

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Stefan Monnier]

> We should stick to a single command and its key binding,

That's exactly the goal of my proposal: users only need to know about
a single command "dired-hide-details-default-mode" and a single custom
var "dired-hide-details-default-mode".  They could ignore the
buffer-local dired-hide-details-mode, since it would be automatically
controlled by dired-hide-details-default-mode.


        Stefan

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Jambunathan K]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> dired-hide-details-default-mode

No need for this even.  See my defcustom + :options suggestions.

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Drew Adams]

> > We should stick to a single command and its key binding,
> 
> That's exactly the goal of my proposal: users only need to know
> about a single command "dired-hide-details-default-mode" and a
> single custom var "dired-hide-details-default-mode".  They could
> ignore the buffer-local dired-hide-details-mode, since it would be
> automatically controlled by dired-hide-details-default-mode.

It's not clear to me just what the behavior is that you are
proposing.  Perhaps we are on the same page; I can't tell.

As I said:

> But if the *same* behavior is offered by your proposal as by mine,
> and if users are not additionally bothered by your proposal, then
> I guess I don't see a problem with it.  (That's an if.)
> 
> IMO:
> 
> 1. The out-of-the-box default behavior should be as I described.
> 
> 2. Users should be able to set their own preferred default behavior
> persistently.  To me, that means a user option.  In any case, it
> means not having to toggle anything interactively just to get the
> preferred behavior as the saved one.
> 
> 3. If the rest of the behavior I described is also provided, no
> problem.
> 
> In sum, it's the behavior I'm interested in.  If the behavior is
> intact then I don't care much how you decide to implement it.

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Lars Ingebrigtsen]

Jambunathan K <kjambunathan@gmail.com> writes:

> Suggest `dired-hide-details-mode' as one of the "available" options to
> `dired-mode-hook'.  (May be through the :options keyword of defcustom.
> Much like what is done for `emacs-lisp-mode-hook').
>
> I have been using dired-hide-details-mode since it has been checked in
> to repo.  I never felt the need for "propagate".  It is hit the toggle
> instead.
>
> I recommend `@' as the keybinding instead of `(' for toggling.

I think it's too late to change the keybinding now, and I don't really
see the need (at this point, five years later) to add the suggestions --
surely people can find this if they want to in the manual?

So I'm closing this bug report.

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

bug#14940: 24.3.50; [PATCH] enhancement for `dired-hide-details-mode'

[Drew Adams]

> Jambunathan K <kjambunathan@gmail.com> writes:
> 
> > Suggest `dired-hide-details-mode' as one of the "available" options to
> > `dired-mode-hook'.  (May be through the :options keyword of defcustom.
> > Much like what is done for `emacs-lisp-mode-hook').
> >
> > I have been using dired-hide-details-mode since it has been checked in
> > to repo.  I never felt the need for "propagate".  It is hit the toggle
> > instead.
> >
> > I recommend `@' as the keybinding instead of `(' for toggling.
> 
> I think it's too late to change the keybinding now, and I don't really
> see the need (at this point, five years later) to add the suggestions --
> surely people can find this if they want to in the manual?
> 
> So I'm closing this bug report.

Interesting that you quote only a mail in the thread
that is a side comment, only peripherally related to
the point of the enhancement request.

I, not Jambunathan, filed the enhancement request and
provided the patch, neither of which are addressed at
all by your closure statement.  Oh well.