Subject: bug#27435: patch for dired-mouse-find-file-other-window

From: Eli Zaretskii <[email-masked]>
Date: Sat, 02 Sep 2017 13:20:06 +0300

> From: Tak Kunihiro <[email-masked]>
> Date: Mon, 28 Aug 2017 09:22:35 +0900
> Cc: 国広卓也 <[email-masked]>,
>  [email-masked]
> 
> With the suggestions, I created two functions.
> 
>  - dired-mouse-find-file (new, this inherits common part)
>  - dired-mouse-find-file-other-window
>  - dired-mouse-find-file-other-frame (new)
> 
> I attach the patch.  This does not change the default behavior.

Thanks.  Some minor comments:

> +(defun dired-mouse-find-file (event &optional find-file-func find-dir-func)
> +  "In Dired, visit the file or directory name you click on.
> +FIND-FILE-FUNC and FIND-DIR-FUNC specify functions to visit the file and directory, respectively."

This last line is too long, please break it into 2.  Also, the doc
string should say explicitly these arguments are optional, and what
they default to if omitted or nil.

> +  (or find-file-func (setq find-file-func 'find-file))
> +  (or find-dir-func (setq find-file-func 'find-alternate-file))

I'm not sure I understand why these particular defaults.  Can you
explain?  In particular, find-alternate-file sounds strange to me.

Finally, 2 nits:

  . please always provide a log message for the changes
  . your patch doesn't apply cleanly, presumably due to some
    difference in whitespace vs the Git repository; please make sure
    your diffs are always against the latest Git version

Thanks.