bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

[Dmitry Gutov]

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

Tags: patch

1. (setq ido-use-filename-at-point t)

2. M-x ido-mode

3. Open a Dired buffer, move point to a directory, press C-x C-f.

4. See the directory name inserted in the prompt both as directory and a
file name.

In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.6)
 of 2014-02-07 on axl
Repository revision: 116299 rgm@gnu.org-20140207032552-3ycw6hai2zl7yynq
Windowing system distributor `The X.Org Foundation', version 11.0.11405000
System Description:	Ubuntu 13.10


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ido-use-filename-at-point.diff --]
[-- Type: text/x-diff, Size: 1173 bytes --]

=== modified file 'lisp/ido.el'
--- lisp/ido.el	2014-01-31 06:34:44 +0000
+++ lisp/ido.el	2014-02-14 05:59:01 +0000
@@ -2375,14 +2375,14 @@
 			  (if (eq ido-use-filename-at-point 'guess)
 			      (ffap-guesser)
 			    (ffap-string-at-point))))
-	       (not (string-match "^http:/" fn))
-	       (let ((absolute-fn (expand-file-name fn)))
-		 (setq d (if (file-directory-p absolute-fn)
-			     (file-name-as-directory absolute-fn)
-			   (file-name-directory absolute-fn))))
-	       (file-directory-p d))
-	  (setq ido-current-directory d)
-	  (setq initial (file-name-nondirectory fn))))))
+	       (not (string-match "^http:/" fn)))
+          (let ((absolute-fn (expand-file-name fn)))
+            (cond
+             ((file-directory-p absolute-fn)
+              (setq ido-current-directory (file-name-as-directory absolute-fn)))
+             ((file-directory-p (file-name-directory absolute-fn))
+              (setq ido-current-directory (file-name-directory absolute-fn))
+              (setq initial (file-name-nondirectory absolute-fn)))))))))
 
     (let (ido-saved-vc-hb
 	  (vc-handled-backends (and (boundp 'vc-handled-backends) vc-handled-backends))

bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

[Leo Liu]

On 2014-02-14 14:05 +0800, Dmitry Gutov wrote:
> Tags: patch
>
> 1. (setq ido-use-filename-at-point t)
>
> 2. M-x ido-mode
>
> 3. Open a Dired buffer, move point to a directory, press C-x C-f.
>
> 4. See the directory name inserted in the prompt both as directory and a
> file name.

I wonder if we could move to a simpler (and maybe better) way of
integrating ffap, for example something like:

diff --git a/lisp/ido.el b/lisp/ido.el
index 9a17c0ca..f1a6b770 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -581,25 +581,6 @@ (defcustom ido-work-directory-list-ignore-regexps nil
   :type '(repeat regexp)
   :group 'ido)
 
-
-(defcustom ido-use-filename-at-point nil
-  "Non-nil means that ido shall look for a filename at point.
-May use `ffap-guesser' to guess whether text at point is a filename.
-If found, use that as the starting point for filename selection."
-  :type '(choice
-	  (const :tag "Disabled" nil)
-	  (const :tag "Guess filename" guess)
-	  (other :tag "Use literal filename" t))
-  :group 'ido)
-
-
-(defcustom ido-use-url-at-point nil
-  "Non-nil means that ido shall look for a URL at point.
-If found, call `find-file-at-point' to visit it."
-  :type 'boolean
-  :group 'ido)
-
-
 (defcustom ido-enable-tramp-completion t
   "Non-nil means that ido shall perform tramp method and server name completion.
 A tramp file name uses the following syntax: /method:user@host:filename."
@@ -2353,35 +2334,6 @@ (defun ido-file-internal (method &optional fallback default prompt item initial
 	    (and (not ido-directory-nonreadable)
 		 (ido-directory-too-big-p ido-current-directory))))
 
-    (when (and (eq item 'file)
-	   (or ido-use-url-at-point ido-use-filename-at-point))
-      (let (fn d)
-	(require 'ffap)
-	;; Duplicate code from ffap-guesser as we want different
-	;; behavior for files and URLs.
-	(cond
-	 ((with-no-warnings
-	    (and ido-use-url-at-point
-		 ffap-url-regexp
-		 (ffap-fixup-url (or (ffap-url-at-point)
-				     (ffap-gopher-at-point)))))
-	  (setq ido-exit 'ffap
-		filename t))
-
-	 ((and ido-use-filename-at-point
-	       (setq fn (with-no-warnings
-			  (if (eq ido-use-filename-at-point 'guess)
-			      (ffap-guesser)
-			    (ffap-string-at-point))))
-	       (not (string-match "^http:/" fn))
-	       (let ((absolute-fn (expand-file-name fn)))
-		 (setq d (if (file-directory-p absolute-fn)
-			     (file-name-as-directory absolute-fn)
-			   (file-name-directory absolute-fn))))
-	       (file-directory-p d))
-	  (setq ido-current-directory d)
-	  (setq initial (file-name-nondirectory fn))))))
-
     (let (ido-saved-vc-hb
 	  (vc-handled-backends (and (boundp 'vc-handled-backends) vc-handled-backends))
 	  (ido-work-directory-index -1)
@@ -2424,9 +2376,6 @@ (defun ido-file-internal (method &optional fallback default prompt item initial
                        (t 'dired))
                  (concat ido-current-directory (or ido-text ""))))
 
-       ((eq ido-exit 'ffap)
-	(find-file-at-point))
-
        ((eq method 'alt-file)
 	(ido-record-work-file filename)
 	(setq default-directory ido-current-directory)
@@ -4186,7 +4135,7 @@ (defun ido-find-file-in-dir (dir)
   (ido-file-internal ido-default-file-method nil dir nil nil nil 'ignore))
 
 ;;;###autoload
-(defun ido-find-file ()
+(defun ido-find-file (&optional use-ffap)
   "Edit file with name obtained via minibuffer.
 The file is displayed according to `ido-default-file-method' -- the
 default is to show it in the same window, unless it is already
@@ -4226,8 +4175,9 @@ (defun ido-find-file ()
 \\[ido-completion-help] Show list of matching files in separate window.
 \\[ido-toggle-ignore] Toggle ignoring files listed in `ido-ignore-files'."
 
-  (interactive)
-  (ido-file-internal ido-default-file-method))
+  (interactive "P")
+  (if use-ffap (find-file-at-point)
+    (ido-file-internal ido-default-file-method)))
 
 ;;;###autoload
 (defun ido-find-file-other-window ()

bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

[Dmitry Gutov]

On 14.02.2014 08:52, Leo Liu wrote:
> +  (interactive "P")
> +  (if use-ffap (find-file-at-point)
> +    (ido-file-internal ido-default-file-method)))

But then in the case of ffap, the file-finding dialog wouldn't use ido, 
would it?

`find-file-at-point' also behaves differently from the current ido when 
point is at a directory: ido enters the directory if it exists, ffap 
treats it just like any other file.

bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

[Leo Liu]

On 2014-02-14 20:18 +0800, Dmitry Gutov wrote:
> But then in the case of ffap, the file-finding dialog wouldn't use
> ido, would it?

ffap conflates file and url reading into ffap-read-file-or-url, which
bypasses read-file-name (it goes directly to read-file-name-internal)
and thus not using ido. So currently ido treats url and file
differently, it handles file itself and delegates url to ffap. Maybe
that separation should be done in ffap, which would make the glue code
in ido minimal. But maybe this is for another bug. Feel free to install
your fix ;)

> `find-file-at-point' also behaves differently from the current ido
> when point is at a directory: ido enters the directory if it exists,
> ffap treats it just like any other file.

Leo

bug#16747: 24.3.50; ido-find-file with ido-use-filename-at-point in dired inserts directory at point twice

[Dmitry Gutov]

On 15.02.2014 11:36, Leo Liu wrote:

 > Maybe
> that separation should be done in ffap, which would make the glue code
> in ido minimal. But maybe this is for another bug.

Yes, that's probably something for after 24.4.

Feel free to install your fix ;)

Done!