GNU bug report logs - #8439
[PATCH] ffap.el -- detect paths with spaces

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8439 in the body.
You can then email your comments to 8439 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jari Aalto <jari.aalto <at> cante.net>
To: submit <at> debbugs.gnu.org
Subject: [PATCH] ffap.el -- detect paths with spaces
Date: Thu, 07 Apr 2011 18:24:39 +0300

[Message part 1 (text/plain, inline)]

Package: emacs
Version: 23.2+1-7
Severity: normal
Tags: patch

An example under Windows:

   (locate-library "ffap.el")C-xC-e
   c:/Program Files/emacs/emacs-23.3/lisp/ffap.el     M-x ffap on line

   => c:/Program

   With the patch, M-x ffap will offer:

   => c:/Program Files/emacs/emacs-23.3/lisp/ffap.el

2011-04-07  Jari Aalto  <jari.aalto <at> cante.net>

        * ffap.el (ffap-string-at-point): Handle paths and file names with
        spaces.

-- System Information
Debian Release: wheezy/sid
  APT Prefers testing
  APT policy: (990, testing) (500, unstable) (1, experimental)
Architecture: amd64
Kernel: Linux picasso 2.6.32-5-amd64 #1 SMP Wed Jan 12 03:40:32 UTC 2011 x86_64 GNU/Linux
Locale: LANG=en_US.UTF-8, LC_ALL=

-- Versions of packages `emacs depends on'.
Depends:
emacs23         23.2+1-7        GNU Emacs is the extensible self-documenting
emacs23-lucid   23.2+1-7        GNU Emacs is the extensible self-documenting
emacs23-nox     23.2+1-7        GNU Emacs is the extensible self-documenting

[0001-lisp-ffap.el-ffap-string-at-point-Support-spaces-in-.patch (text/x-diff, inline)]

From 0706445da3f7e10aceef8b371d458deb4ec1c39d Mon Sep 17 00:00:00 2001
From: Jari Aalto <jari.aalto <at> cante.net>
Date: Thu, 7 Apr 2011 18:20:17 +0300
Subject: [PATCH] lisp/ffap.el: (ffap-string-at-point): Support spaces in paths
Organization: Private
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit


Signed-off-by: Jari Aalto <jari.aalto <at> cante.net>
---
 lisp/ffap.el |   47 +++++++++++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 97105ed..dcc16f2 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1006,24 +1006,35 @@ If MODE is not found, we use `file' instead of MODE.
 If the region is active, return a string from the region.
 Sets `ffap-string-at-point' and `ffap-string-at-point-region'."
   (let* ((args
-	  (cdr
-	   (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
-	       (assq 'file ffap-string-at-point-mode-alist))))
-	 (pt (point))
-	 (str
-	  (if (and transient-mark-mode mark-active)
-	      (buffer-substring
-	       (setcar ffap-string-at-point-region (region-beginning))
-	       (setcar (cdr ffap-string-at-point-region) (region-end)))
-	    (buffer-substring
-	     (save-excursion
-	       (skip-chars-backward (car args))
-	       (skip-chars-forward (nth 1 args) pt)
-	       (setcar ffap-string-at-point-region (point)))
-	     (save-excursion
-	       (skip-chars-forward (car args))
-	       (skip-chars-backward (nth 2 args) pt)
-	       (setcar (cdr ffap-string-at-point-region) (point)))))))
+          (cdr
+           (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
+               (assq 'file ffap-string-at-point-mode-alist))))
+         (pt (point))
+	  space-p
+	   end
+         (str
+          (if (and transient-mark-mode mark-active)
+              (buffer-substring
+               (setcar ffap-string-at-point-region (region-beginning))
+               (setcar (cdr ffap-string-at-point-region) (region-end)))
+            (buffer-substring
+             (save-excursion
+               (skip-chars-backward (car args))
+               (skip-chars-forward (nth 1 args) pt)
+	              ;; Paths may contains spaces, check those
+	              ;; FIXME: may need better tuning. Currenly matches "/paths like/exam ple.txt"
+	              (if (looking-at ".*/.* [^[:space:]]*/.*[^[:space:]]")
+			     (setq space-p (match-end 0)))
+               (setcar ffap-string-at-point-region (point)))
+             (save-excursion
+               (skip-chars-forward (car args))
+               (skip-chars-backward (nth 2 args) pt)
+	              (setq end (point))
+		             (if (and space-p
+				      (> space-p end)
+				      (memq mode '(file nil)))
+				    (setq end space-p))
+               (setcar (cdr ffap-string-at-point-region) end))))))
     (set-text-properties 0 (length str) nil str)
     (setq ffap-string-at-point str)))
 
-- 
1.7.4.1

Message #8 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: Jari Aalto <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org
Subject: Re: [PATCH] ffap.el -- detect paths with spaces
Date: Mon, 04 Jul 2011 01:29:16 +0200

Jari Aalto <jari.aalto <at> cante.net> writes:

> An example under Windows:
>
>    (locate-library "ffap.el")C-xC-e
>    c:/Program Files/emacs/emacs-23.3/lisp/ffap.el     M-x ffap on line
>
>    => c:/Program
>
>    With the patch, M-x ffap will offer:
>
>    => c:/Program Files/emacs/emacs-23.3/lisp/ffap.el

[...]

> +	              ;; FIXME: may need better tuning. Currenly matches "/paths like/exam ple.txt"
> +	              (if (looking-at ".*/.* [^[:space:]]*/.*[^[:space:]]")

Won't this be more problematic than helpful?  There may be more matches
of stuff that's not part of the file name.  And spaces in file names
aren't that common in non-Windowsey environments...

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

Message #13 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Jari Aalto <jari.aalto <at> cante.net>
To: 8439 <at> debbugs.gnu.org
Cc: Yigal Hochberg <hochberg <at> dmhsoftware.com>
Subject: Re: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 10:39:36 +0300

With some more field testing, the following patch works better.

  1. New variable ffap-paths-with-spaces controls if the behavior is in use.
     By default it is set to "t" in Windows/Cygwin where spaces in paths
     are encountered.

  2. The "path skipping" is based on finding longest distance:

     - from first "drive letter", "/" or "\"
     - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names

  3. In Cygwin Emacs the paths are expected to be in "/cygdrive/<letter>/"
     format. This is handled as a last step.

Test by placing cursor anywhere of these path names and evaluating with
"M-:" the lisp code "(ffap-string-at-point)" over them:

  ;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program here.txt
  ;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
  ;; c:\Program Files\Freescale\CW for MPC55xx and MPC56xx 2.10\PowerPC_EABI_Tools\Command_Line_Tools\CLT_Usage_Notes.txt

It is not possible to test the above paths with "M-x ffap" as the code by
design starts to chop path components if the path does not exist in current
environment. See:

  (defun ffap-file-at-point ()

  ...near the end....

  ;; Try all parent directories by deleting the trailing directory
  ;; name until existing directory is found or name stops changing

I'm sure the patch is not perfect as it's hard to come by a strategy to
deal with spaces but it appears to give good results. I think users' would
benefit from the space detection support.

Thanks for Yigal Hochberg for testing the functionality in Windows,
Jari

Message #16 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Jari Aalto <jari.aalto <at> cante.net>
To: 8439 <at> debbugs.gnu.org
Subject: Re: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 11:35:17 +0300

[Message part 1 (text/plain, inline)]

Patch for http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8439#13

[0001-lisp-ffap.el-ffap-string-at-point-v2-support-spaces.patch (text/x-diff, inline)]

From 9d912ef5fe082c81917cf65bbacbf4d23e1cd9f3 Mon Sep 17 00:00:00 2001
From: Jari Aalto <jari.aalto <at> cante.net>
Date: Fri, 19 Oct 2012 10:15:50 +0300
Subject: [PATCH] lisp/ffap.el: (ffap-string-at-point): v2 support spaces
Organization: Private
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Signed-off-by: Jari Aalto <jari.aalto <at> cante.net>
---
 lisp/ffap.el |   87 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 19 deletions(-)

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 4c75609..a74f85b 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1036,6 +1036,17 @@ possibly a major-mode name, or one of the symbol
   ;; Added at suggestion of RHOGEE (for ff-paths), 7/24/95.
   "Last string returned by `ffap-string-at-point'.")
 
+;; Test cases:
+;;
+;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program here.txt
+;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
+;;
+(defvar ffap-paths-with-spaces
+  (or (memq system-type '(ms-dos windows-nt))
+      (string-match "cygwin" (emacs-version)))
+  "If non-nil, look for paths with spaces in `ffap-string-at-point'.
+Enabled in W32 and Cygwin by default.")
+
 (defun ffap-string-at-point (&optional mode)
   "Return a string of characters from around point.
 MODE (defaults to value of `major-mode') is a symbol used to look up string
@@ -1043,25 +1054,63 @@ syntax parameters in `ffap-string-at-point-mode-alist'.
 If MODE is not found, we use `file' instead of MODE.
 If the region is active, return a string from the region.
 Sets `ffap-string-at-point' and `ffap-string-at-point-region'."
-  (let* ((args
-	  (cdr
-	   (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
-	       (assq 'file ffap-string-at-point-mode-alist))))
-	 (pt (point))
-	 (str
-	  (if (and transient-mark-mode mark-active)
-	      (buffer-substring
-	       (setcar ffap-string-at-point-region (region-beginning))
-	       (setcar (cdr ffap-string-at-point-region) (region-end)))
-	    (buffer-substring
-	     (save-excursion
-	       (skip-chars-backward (car args))
-	       (skip-chars-forward (nth 1 args) pt)
-	       (setcar ffap-string-at-point-region (point)))
-	     (save-excursion
-	       (skip-chars-forward (car args))
-	       (skip-chars-backward (nth 2 args) pt)
-	       (setcar (cdr ffap-string-at-point-region) (point)))))))
+  (let* ((cygwin-p (string-match "cygwin" (emacs-version)))
+	 (args
+          (cdr
+           (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
+               (assq 'file ffap-string-at-point-mode-alist))))
+         (pt (point))
+	  space-p
+	   end
+         (str
+          (if (and transient-mark-mode mark-active)
+              (buffer-substring
+               (setcar ffap-string-at-point-region (region-beginning))
+               (setcar (cdr ffap-string-at-point-region) (region-end)))
+            (buffer-substring
+             (save-excursion
+	       (if (and ffap-paths-with-spaces
+			(memq mode '(nil file)))
+		   (if (string-match "^[ \t]*$"
+				     (buffer-substring (line-beginning-position)
+						       (point)))
+		       ;; Nothing interesting before point. Move to the first character
+		       (skip-chars-forward " \t" (line-end-position))
+		     ;; If at colon, move a little forward so that next
+		     ;; `re-search-backward' can position at drive letter.
+		     (if (looking-at ":/")
+			 (forward-char 1))
+		     ;; Skip until drive path start or patch start letter
+		     (while (re-search-backward "[a-zA-Z]:[\\\\/]\\|[/\\\\]"
+						     (line-beginning-position) t)
+		       (goto-char (match-beginning 0)))))
+               (skip-chars-backward (car args))
+               (skip-chars-forward (nth 1 args) pt)
+	       (when (and ffap-paths-with-spaces
+			  (memq mode '(nil file)))
+		 ;; Paths may contains spaces, allow those
+		 (if (looking-at
+		      "[^\t\r\n]*[/\\\\][^][<>()\"';:|\t\r\n]*[^][<>()\"';:|\r\n[:space:]]")
+		     (setq space-p (match-end 0))))
+               (setcar ffap-string-at-point-region (point)))
+             (save-excursion
+               (skip-chars-forward (car args))
+               (skip-chars-backward (nth 2 args) pt)
+	              (setq end (point))
+		             (if (and space-p
+				      (> space-p end)
+				      (memq mode '(file nil)))
+				    (setq end space-p))
+               (setcar (cdr ffap-string-at-point-region) end))))))
+    ;; Under Cygwin, convert drive letters in paths.
+    (when (and cygwin-p
+               (memq mode '(nil file))
+               (string-match "^\\([a-zA-Z]\\):[/\\\\]\\(.*\\)" str))
+      (let ((drive (downcase (match-string 1 str)))
+            (path (match-string 2 str)))
+        (setq str (format "/cygdrive/%s/%s"
+                          drive
+                          (replace-regexp-in-string "[\\\\]" "/" path)))))
     (set-text-properties 0 (length str) nil str)
     (setq ffap-string-at-point str)))
 
-- 
1.7.10.4

Message #19 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jari Aalto <jari.aalto <at> cante.net>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 11:49:25 +0200

> From: Jari Aalto <jari.aalto <at> cante.net>
> Date: Fri, 19 Oct 2012 10:39:36 +0300
> Cc: Yigal Hochberg <hochberg <at> dmhsoftware.com>
> 
> 
> With some more field testing, the following patch works better.
> 
>   1. New variable ffap-paths-with-spaces controls if the behavior is in use.
>      By default it is set to "t" in Windows/Cygwin where spaces in paths
>      are encountered.

GNU Coding Standards frown upon using "path" for anything except
PATH-style lists of directories.  Use "file name" or "leading
directories" instead.

>   2. The "path skipping" is based on finding longest distance:
> 
>      - from first "drive letter", "/" or "\"
>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names

What will this do to text such as the one below?

 C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.

>   3. In Cygwin Emacs the paths are expected to be in "/cygdrive/<letter>/"
>      format.

What about Posix-standard /foo/bar/baz, which is the usual case on
Cygwin?

Message #22 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: jari <jari.aalto <at> cante.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 13:05:56 +0300

On 2012-10-19 11:49, Eli Zaretskii wrote:
| > 
| >   1. New variable ffap-paths-with-spaces controls if the behavior is in use.
| >      By default it is set to "t" in Windows/Cygwin where spaces in paths
| >      are encountered.
| 
| GNU Coding Standards frown upon using "path" for anything except
| PATH-style lists of directories.  Use "file name" or "leading
| directories" instead.
| 
| >   2. The "path skipping" is based on finding longest distance:
| > 
| >      - from first "drive letter", "/" or "\"
| >      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
| 
| What will this do to text such as the one below?
| 
|  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.

User can enable/disable if spaces are taken into account. If enabled,
the result is:

  "C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1"

If disabled, depending on cursor position:

  "C:\emacs\bin\prog1.exe"
  "/usr/bin/prog1"

If you have strategiy to make it better, please suggest.

| >   3. In Cygwin Emacs the paths are expected to be in "/cygdrive/<letter>/"
| >      format.
| 
| What about Posix-standard /foo/bar/baz, which is the usual case on
| Cygwin?

Only "<drive letter>:" is converted into "/cygdrive/<drive letter>" under
Cygwin.

Jari

Message #25 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jari <jari.aalto <at> cante.net>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 13:22:10 +0200

> Date: Fri, 19 Oct 2012 13:05:56 +0300
> From: jari <jari.aalto <at> cante.net>
> Cc: 8439 <at> debbugs.gnu.org, hochberg <at> dmhsoftware.com
> 
> | What will this do to text such as the one below?
> | 
> |  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
> 
> User can enable/disable if spaces are taken into account. If enabled,
> the result is:
> 
>   "C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1"

Why would it exclude the " on Unix." part?

> If you have strategiy to make it better, please suggest.

(I don't use ffap, so take the below with a grain of salt.)

Several things come to mind:

 . You could modify the heuristics depending on the major mode.  E.g.,
   if it's Text or anything derived from text, file names with spaces
   will generally be delimited by quotes.

 . You could by default stop "skipping" when you see common words like
   "on", "and", "or", "and/or", etc.

 . You could by default stop when the slash flavor changes, like first
   \, then / (see my example).

 . At least in text modes, punctuation (with the notable exception of
   a period) will not normally be part of a file name.

> | >   3. In Cygwin Emacs the paths are expected to be in "/cygdrive/<letter>/"
> | >      format.
> | 
> | What about Posix-standard /foo/bar/baz, which is the usual case on
> | Cygwin?
> 
> Only "<drive letter>:" is converted into "/cygdrive/<drive letter>" under
> Cygwin.

Sorry, I don't follow.  You said "In Cygwin Emacs the paths are
_expected_ to be", so I assumed you mean the file names on Cygwin are
_required_ to be in the /cygdrive/x/foo form.  Maybe I misunderstood
what you meant.

Message #28 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Nicolas Richard" <theonewiththeevillook <at> yahoo.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 14:15:20 +0200

jari <jari.aalto <at> cante.net> writes:
> On 2012-10-19 11:49, Eli Zaretskii wrote:
> |  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.

> If you have strategiy to make it better, please suggest.

From the example, I'd suggest that if there is a period followed by up
to three letters and then a space, the filename ends before that space.
From what I see, most filenames which have an extension don't have more
text after the extension (with the exception of another extension, which
is why I suggest that there must be a final space). Would that fit  ?

-- 
Nico.

Message #31 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Yigal Hochberg <hochberg <at> dmhsoftware.com>
To: jari <jari.aalto <at> cante.net>
Cc: Yigal Hochberg <hochberg <at> dmhsoftware.com>, Eli Zaretskii <eliz <at> gnu.org>,
	8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 07:15:00 -0400

[Message part 1 (text/plain, inline)]

Hi Jari, Eli:

A thank you note for Jari.

I asked Jari if he could improve ffap to match these Windows paths like
this:

c:/Program Files/Freescale/CW for MPC55xx and MPC56xx
2.10/PowerPC_EABI_Tools/Command_Line_Tools/CLT_EABI_PPC_Tools_Notes.txt
c:\Program Files\Freescale\CW for MPC55xx and MPC56xx
2.10\PowerPC_EABI_Tools\Command_Line_Tools\CLT_Usage_Notes.txt
c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program
Files/Hummingbird/Connectivity/14.00/Default User/PerUser Settings.ini
c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program
Files\Hummingbird\Connectivity\14.00\Default User\PerUser Settings.ini
c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program
Files/Hummingbird/
c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program
Files\Hummingbird\

From a user view this is a very helpful and useful solution to get to these
files/dirs in one keystroke.

Thanks to Jari for the hard work.

I will report if I find issues.

Regards,

Yigal




On Fri, Oct 19, 2012 at 6:05 AM, jari <jari.aalto <at> cante.net> wrote:

> On 2012-10-19 11:49, Eli Zaretskii wrote:
> | >
> | >   1. New variable ffap-paths-with-spaces controls if the behavior is
> in use.
> | >      By default it is set to "t" in Windows/Cygwin where spaces in
> paths
> | >      are encountered.
> |
> | GNU Coding Standards frown upon using "path" for anything except
> | PATH-style lists of directories.  Use "file name" or "leading
> | directories" instead.
> |
> | >   2. The "path skipping" is based on finding longest distance:
> | >
> | >      - from first "drive letter", "/" or "\"
> | >      - until last "/", "\" followed by last
> non-matching-char-not-usually-found-in-file-names
> |
> | What will this do to text such as the one below?
> |
> |  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
>
> User can enable/disable if spaces are taken into account. If enabled,
> the result is:
>
>   "C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1"
>
> If disabled, depending on cursor position:
>
>   "C:\emacs\bin\prog1.exe"
>   "/usr/bin/prog1"
>
> If you have strategiy to make it better, please suggest.
>
> | >   3. In Cygwin Emacs the paths are expected to be in
> "/cygdrive/<letter>/"
> | >      format.
> |
> | What about Posix-standard /foo/bar/baz, which is the usual case on
> | Cygwin?
>
> Only "<drive letter>:" is converted into "/cygdrive/<drive letter>" under
> Cygwin.
>
> Jari
>



-- 
Yigal

[Message part 2 (text/html, inline)]

Message #34 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org,
	Jari Aalto <jari.aalto <at> cante.net>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 00:47:55 +0300

On Fri, Oct 19, 2012 at 12:49 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Jari Aalto <jari.aalto <at> cante.net>
>> Date: Fri, 19 Oct 2012 10:39:36 +0300
>> Cc: Yigal Hochberg <hochberg <at> dmhsoftware.com>
>>
>>
>> With some more field testing, the following patch works better.
>>
>>   1. New variable ffap-paths-with-spaces controls if the behavior is in use.
>>      By default it is set to "t" in Windows/Cygwin where spaces in paths
>>      are encountered.
>
> GNU Coding Standards frown upon using "path" for anything except
> PATH-style lists of directories.  Use "file name" or "leading
> directories" instead.
>
>>   2. The "path skipping" is based on finding longest distance:
>>
>>      - from first "drive letter", "/" or "\"
>>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
>
> What will this do to text such as the one below?
>
>  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.

Eli, excuse me very much, but you cheating.  Even under Unix (where
spaces inside filenames are very unusual) people instinctively trying
heavy to distinguish/decorate them from outside/descriptive text.  The
only one problem that there no consensus about how to do it.  What
characters pair to use? <>? ""? ''?  But anyway just separate filename
with possible spaces just by spaces...  It's out of my ability to
imagine.  Therefore, I would to discard this example as rare.  After
all, any heuristic have his own false positives (and it is the reason,
why I said "cheating": for every heuristic it's always possible to
imagine a situation/input data when heuristic will fail).

But, if there no way to make false positives amount small enough, I
would to propose an idea for extension: find-file-at-region().  You
just select a region of text, and Emacs tries to interprets it as
default value for find-file().

>
>>   3. In Cygwin Emacs the paths are expected to be in "/cygdrive/<letter>/"
>>      format.
>
> What about Posix-standard /foo/bar/baz, which is the usual case on
> Cygwin?
>
>
>



-- 
Andrew W. Nosenko <andrew.w.nosenko <at> gmail.com>

Message #37 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 00:23:59 +0200

> Date: Sat, 20 Oct 2012 00:47:55 +0300
> From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
> Cc: Jari Aalto <jari.aalto <at> cante.net>, hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
> 
> >>   2. The "path skipping" is based on finding longest distance:
> >>
> >>      - from first "drive letter", "/" or "\"
> >>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
> >
> > What will this do to text such as the one below?
> >
> >  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
> 
> Eli, excuse me very much, but you cheating.

Actually, I had this weird idea that maybe the suggested heuristics
could be improved.  Now I'm sorry that I have spoken.

Message #40 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Yigal Hochberg <hochberg <at> dmhsoftware.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org,
	"Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 18:36:48 -0400 (EDT)

Eli, Andrew:

What Jari did is an excellent solution (increment cheeking if file exists).
From a user point of view it is very powerful and useful.

Marking a region is trivial but way too costly for me. I want to be one keystroke 
away from getting this file. On Windows I use these type of paths:

c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/Connectivity/14.00/Default User/PerUser Settings.ini
c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\Connectivity\14.00\Default User\PerUser Settings.ini

c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\

With Jari solution I can get to the file/dir in one keystroke.
I think that this is the power of emacs. No?

Thanks,

Yigal


On Fri, Oct 19, 2012 at 6:23 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Sat, 20 Oct 2012 00:47:55 +0300
>> From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
>> Cc: Jari Aalto <jari.aalto <at> cante.net>, hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
>> 
>> >>   2. The "path skipping" is based on finding longest distance:
>> >>
>> >>      - from first "drive letter", "/" or "\"
>> >>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
>> >
>> > What will this do to text such as the one below?
>> >
>> >  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
>> 
>> Eli, excuse me very much, but you cheating.
>
>Actually, I had this weird idea that maybe the suggested heuristics
>could be improved.  Now I'm sorry that I have spoken.
>

Message #43 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
To: Yigal Hochberg <hochberg <at> dmhsoftware.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 8439 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 01:57:43 +0300

On Sat, Oct 20, 2012 at 1:36 AM, Yigal Hochberg
<hochberg <at> dmhsoftware.com> wrote:
> Eli, Andrew:
>
> What Jari did is an excellent solution (increment cheeking if file exists).
> From a user point of view it is very powerful and useful.
>
> Marking a region is trivial but way too costly for me. I want to be one keystroke
> away from getting this file.

O, sorry, sorry and sorry again!  I ever didn't mind that my
"fallback" proposal can sound so seriously!  It was intended as a very
last resort, if there will be no reasonable way to avoid 50%/50% false
positive/negative ratio.  Logic was very simple: in any way to mark a
region and call a function is faster than to mark a region, copy, call
a function, paste and, optionally if copied pathname is relative, move
to the beginning of the pasted pathname and then to kill to the
beginning of minibuffer.

> On Windows I use these type of paths:
>
> c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/Connectivity/14.00/Default User/PerUser Settings.ini
> c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\Connectivity\14.00\Default User\PerUser Settings.ini
>
> c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
> c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\
>
> With Jari solution I can get to the file/dir in one keystroke.
> I think that this is the power of emacs. No?
>
> Thanks,
>
> Yigal
>
>
> On Fri, Oct 19, 2012 at 6:23 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> Date: Sat, 20 Oct 2012 00:47:55 +0300
>>> From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
>>> Cc: Jari Aalto <jari.aalto <at> cante.net>, hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
>>>
>>> >>   2. The "path skipping" is based on finding longest distance:
>>> >>
>>> >>      - from first "drive letter", "/" or "\"
>>> >>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
>>> >
>>> > What will this do to text such as the one below?
>>> >
>>> >  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
>>>
>>> Eli, excuse me very much, but you cheating.
>>
>>Actually, I had this weird idea that maybe the suggested heuristics
>>could be improved.  Now I'm sorry that I have spoken.
>>



-- 
Andrew W. Nosenko <andrew.w.nosenko <at> gmail.com>

Message #46 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Yigal Hochberg <hochberg <at> dmhsoftware.com>
To: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
Cc: Yigal Hochberg <hochberg <at> dmhsoftware.com>, Eli Zaretskii <eliz <at> gnu.org>,
	8439 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 19:24:24 -0400 (EDT)

Sorry Andrew,

Marking a region is missing the whole point of ffap.
One single keystroke to get to what you wanna do - no more.

Yigal


On Fri, Oct 19, 2012 at 6:57 PM, Andrew W. Nosenko <andrew.w.nosenko <at> gmail.com> wrote:
>On Sat, Oct 20, 2012 at 1:36 AM, Yigal Hochberg
><hochberg <at> dmhsoftware.com> wrote:
>> Eli, Andrew:
>>
>> What Jari did is an excellent solution (increment cheeking if file exists).
>> From a user point of view it is very powerful and useful.
>>
>> Marking a region is trivial but way too costly for me. I want to be one keystroke
>> away from getting this file.
>
>O, sorry, sorry and sorry again!  I ever didn't mind that my
>"fallback" proposal can sound so seriously!  It was intended as a very
>last resort, if there will be no reasonable way to avoid 50%/50% false
>positive/negative ratio.  Logic was very simple: in any way to mark a
>region and call a function is faster than to mark a region, copy, call
>a function, paste and, optionally if copied pathname is relative, move
>to the beginning of the pasted pathname and then to kill to the
>beginning of minibuffer.
>
>> On Windows I use these type of paths:
>>
>> c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/Connectivity/14.00/Default User/PerUser Settings.ini
>> c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\Connectivity\14.00\Default User\PerUser Settings.ini
>>
>> c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
>> c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\
>>
>> With Jari solution I can get to the file/dir in one keystroke.
>> I think that this is the power of emacs. No?
>>
>> Thanks,
>>
>> Yigal
>>
>>
>> On Fri, Oct 19, 2012 at 6:23 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>> Date: Sat, 20 Oct 2012 00:47:55 +0300
>>>> From: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
>>>> Cc: Jari Aalto <jari.aalto <at> cante.net>, hochberg <at> dmhsoftware.com, 8439 <at> debbugs.gnu.org
>>>>
>>>> >>   2. The "path skipping" is based on finding longest distance:
>>>> >>
>>>> >>      - from first "drive letter", "/" or "\"
>>>> >>      - until last "/", "\" followed by last non-matching-char-not-usually-found-in-file-names
>>>> >
>>>> > What will this do to text such as the one below?
>>>> >
>>>> >  C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
>>>>
>>>> Eli, excuse me very much, but you cheating.
>>>
>>>Actually, I had this weird idea that maybe the suggested heuristics
>>>could be improved.  Now I'm sorry that I have spoken.
>>>
>
>
>
>-- 
>Andrew W. Nosenko <andrew.w.nosenko <at> gmail.com>
>

Message #49 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jari Aalto <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 21:44:02 -0400

A few comments about your patch below.


        Stefan


> +  (let* ((cygwin-p (string-match "cygwin" (emacs-version)))

Test system-type instead.

> +	       (if (and ffap-paths-with-spaces
> +			(memq mode '(nil file)))
> +		   (if (string-match "^[ \t]*$"
> +				     (buffer-substring (line-beginning-position)
> +						       (point)))

Better match the buffer (e.g. with forward-line + looking-at, or in this
case with skip-chars-backward + bolp) than extract a part of the buffer
and then apply string-match to it.

> +		       ;; Nothing interesting before point. Move to the first character

Please terminate your comments with proper punctuation.

> +		       (skip-chars-forward " \t" (line-end-position))
> +		     ;; If at colon, move a little forward so that next
> +		     ;; `re-search-backward' can position at drive letter.
> +		     (if (looking-at ":/")
> +			 (forward-char 1))
> +		     ;; Skip until drive path start or patch start letter
> +		     (while (re-search-backward "[a-zA-Z]:[\\\\/]\\|[/\\\\]"
> +						     (line-beginning-position) t)
> +		       (goto-char (match-beginning 0)))))

This code seems to assume Windows-style file names, whereas none of the
var names nor comments mention anything about this assumption.

I'm not sure we should make this assumption, but it's probably OK to
consider that spaces only appear in Windows-style file names.  Just make
this reasoning explicit in a comment.

> +	       (when (and ffap-paths-with-spaces
> +			  (memq mode '(nil file)))
> +		 ;; Paths may contains spaces, allow those
> +		 (if (looking-at
> +		      "[^\t\r\n]*[/\\\\][^][<>()\"';:|\t\r\n]*[^][<>()\"';:|\r\n[:space:]]")
> +		     (setq space-p (match-end 0))))

This regexp needs some explanation.  Also I don't like the control&data
flow very much, here, so you need to compensate this complexity by
explaining clearly what this `space-p' is supposed to represent.
Same about `end'.

> +	              (setq end (point))
> +		             (if (and space-p
> +				      (> space-p end)
> +				      (memq mode '(file nil)))
> +				    (setq end space-p))

As mentioned, the control&data flow is pretty ugly, but even then
I think the (memq mode '(file nil)) is redundant because space-p can
only be non-nil if (memq mode '(file nil)) is non-nil.

> +    ;; Under Cygwin, convert drive letters in paths.
> +    (when (and cygwin-p
> +               (memq mode '(nil file))
> +               (string-match "^\\([a-zA-Z]\\):[/\\\\]\\(.*\\)" str))
> +      (let ((drive (downcase (match-string 1 str)))
> +            (path (match-string 2 str)))
> +        (setq str (format "/cygdrive/%s/%s"
> +                          drive
> +                          (replace-regexp-in-string "[\\\\]" "/" path)))))

This shouldn't be here: the right way to do it  is to make the Cygwin
Emacs accept Windows-style file name.

Message #52 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
Cc: hochberg <at> dmhsoftware.com, Eli Zaretskii <eliz <at> gnu.org>,
	8439 <at> debbugs.gnu.org, Jari Aalto <jari.aalto <at> cante.net>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 21:45:25 -0400

>> C:\emacs\bin\prog1.exe on Windows or /usr/bin/prog1 on Unix.
> Eli, excuse me very much, but you cheating.

I don't see the relationship with cheating.  Yes, his example is
probably not too important, but I think if we want to add support for
spaces we first need to decide which cases we want to handle, since the
general case is simply impossible.

Here are some disorganized thoughts:
- many of the examples shown in this thread have to do with "one file
  name per line".  But if noone tells us beforehand that the file name
  goes to EOL, handling these well will unavoidably end up with too many
  false positives.
- we should think of good hints for beginning/end of file names.
  E.g. I think if we find something that matches "['\">,.][ \n]", we
  should probably assume it's the end of the file name (EOFN).
  Or " /" is probably a BOFN, whereas "/ " is an EOFN.
- maybe if the separators are forward slashes, we should assume that
  there's no space in the file name.

> But, if there no way to make false positives amount small enough, I
> would to propose an idea for extension: find-file-at-region().  You
> just select a region of text, and Emacs tries to interprets it as
> default value for find-file().

M-w C-x C-f C-y already handles this case.


        Stefan

Message #55 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Colascione <dancol <at> dancol.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8439 <at> debbugs.gnu.org, Jari Aalto <jari.aalto <at> cante.net>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Fri, 19 Oct 2012 18:49:10 -0700

[Message part 1 (text/plain, inline)]

On 10/19/2012 6:44 PM, Stefan Monnier wrote:
> A few comments about your patch below.
> 
> 
>         Stefan
> 
> 
>> +  (let* ((cygwin-p (string-match "cygwin" (emacs-version)))
> 
> Test system-type instead.
> 
>> +	       (if (and ffap-paths-with-spaces
>> +			(memq mode '(nil file)))
>> +		   (if (string-match "^[ \t]*$"
>> +				     (buffer-substring (line-beginning-position)
>> +						       (point)))
> 
> Better match the buffer (e.g. with forward-line + looking-at, or in this
> case with skip-chars-backward + bolp) than extract a part of the buffer
> and then apply string-match to it.
> 
>> +		       ;; Nothing interesting before point. Move to the first character
> 
> Please terminate your comments with proper punctuation.
> 
>> +		       (skip-chars-forward " \t" (line-end-position))
>> +		     ;; If at colon, move a little forward so that next
>> +		     ;; `re-search-backward' can position at drive letter.
>> +		     (if (looking-at ":/")
>> +			 (forward-char 1))
>> +		     ;; Skip until drive path start or patch start letter
>> +		     (while (re-search-backward "[a-zA-Z]:[\\\\/]\\|[/\\\\]"
>> +						     (line-beginning-position) t)
>> +		       (goto-char (match-beginning 0)))))
> 
> This code seems to assume Windows-style file names, whereas none of the
> var names nor comments mention anything about this assumption.
> 
> I'm not sure we should make this assumption, but it's probably OK to
> consider that spaces only appear in Windows-style file names.  Just make
> this reasoning explicit in a comment.
> 
>> +	       (when (and ffap-paths-with-spaces
>> +			  (memq mode '(nil file)))
>> +		 ;; Paths may contains spaces, allow those
>> +		 (if (looking-at
>> +		      "[^\t\r\n]*[/\\\\][^][<>()\"';:|\t\r\n]*[^][<>()\"';:|\r\n[:space:]]")
>> +		     (setq space-p (match-end 0))))
> 
> This regexp needs some explanation.  Also I don't like the control&data
> flow very much, here, so you need to compensate this complexity by
> explaining clearly what this `space-p' is supposed to represent.
> Same about `end'.
> 
>> +	              (setq end (point))
>> +		             (if (and space-p
>> +				      (> space-p end)
>> +				      (memq mode '(file nil)))
>> +				    (setq end space-p))
> 
> As mentioned, the control&data flow is pretty ugly, but even then
> I think the (memq mode '(file nil)) is redundant because space-p can
> only be non-nil if (memq mode '(file nil)) is non-nil.
> 
>> +    ;; Under Cygwin, convert drive letters in paths.
>> +    (when (and cygwin-p
>> +               (memq mode '(nil file))
>> +               (string-match "^\\([a-zA-Z]\\):[/\\\\]\\(.*\\)" str))
>> +      (let ((drive (downcase (match-string 1 str)))
>> +            (path (match-string 2 str)))
>> +        (setq str (format "/cygdrive/%s/%s"
>> +                          drive
>> +                          (replace-regexp-in-string "[\\\\]" "/" path)))))
> 
> This shouldn't be here: the right way to do it  is to make the Cygwin
> Emacs accept Windows-style file name.

Stefan is right. But if you do want to convert Cygwin paths in some other
context, please use cygwin-convert-path-from-windows and
cygwin-convert-path-to-windows instead of hard-coding a particular mapping. (Not
everyone's Cygwin drive prefix is "/cygdrive".)

[signature.asc (application/pgp-signature, attachment)]

Message #58 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: jari <jari.aalto <at> cante.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8439 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v3)
Date: Sat, 20 Oct 2012 10:56:01 +0300

[Message part 1 (text/plain, inline)]

On 2012-10-19 21:44, Stefan Monnier wrote:
| A few comments about your patch below.

Included. I also worked on Eli's ideas about detecting "/" or "\" near
point.

The patch has increased significantly as there are no easy way to deal
with spaces. For test cases I've used, it looks promising.

Here is also ffap.el with the patch included for "live" testing.

Jari

[0001-lisp-ffap.el-Detect-patchs-with-spaces.patch (text/x-diff, attachment)]

[ffap--jari-2012-10-20-git-ec3edd7.el (text/plain, attachment)]

Message #61 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8439 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 10:11:56 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Date: Fri, 19 Oct 2012 21:44:02 -0400
> Cc: 8439 <at> debbugs.gnu.org
> 
> > +    ;; Under Cygwin, convert drive letters in paths.
> > +    (when (and cygwin-p
> > +               (memq mode '(nil file))
> > +               (string-match "^\\([a-zA-Z]\\):[/\\\\]\\(.*\\)" str))
> > +      (let ((drive (downcase (match-string 1 str)))
> > +            (path (match-string 2 str)))
> > +        (setq str (format "/cygdrive/%s/%s"
> > +                          drive
> > +                          (replace-regexp-in-string "[\\\\]" "/" path)))))
> 
> This shouldn't be here: the right way to do it  is to make the Cygwin
> Emacs accept Windows-style file name.

Someone said in another discussion that Cygwin already does accept
Windows-style file names.

Message #64 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: jari <jari.aalto <at> cante.net>
To: 8439 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 20 Oct 2012 11:45:51 +0300

[Message part 1 (text/plain, inline)]

On 2012-10-20 10:11:56 Eli Zaretskii:
| Someone said in another discussion that Cygwin already does accept
| Windows-style file names.

Cygwin Emacs 24.2.1:

   C-x C-f c:/test.txt
   => Use M-x make-directory RET RET to create the directory and its parents

   C-x C-f c:\test.txt
   => Use M-x make-directory RET RET to create the directory and its parents

| On 2012-10-20 18:49:10 Daniel Colascione:
| please use cygwin-convert-path-from-windows and

In this patch (compared to previous v3):

   - remove functions related to Cygwin and use cygwin-convert-path-from-windows

   - obey END parameter in ffap-search-backward-file-end as originally
     intended

   - correct "path-separator" term into "dir-separator" everywhere

Jari

[0001-lisp-ffap.el-Detect-patchs-with-spaces-v4.patch (text/x-diff, attachment)]

[ffap--jari-2012-10-20-1128-git-20d87fd.el (text/plain, attachment)]

Message #67 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jari <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 20 Oct 2012 12:20:46 +0200

> Date: Sat, 20 Oct 2012 11:45:51 +0300
> From: jari <jari.aalto <at> cante.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>
> 
> On 2012-10-20 10:11:56 Eli Zaretskii:
> | Someone said in another discussion that Cygwin already does accept
> | Windows-style file names.
> 
> Cygwin Emacs 24.2.1:
> 
>    C-x C-f c:/test.txt
>    => Use M-x make-directory RET RET to create the directory and its parents
> 
>    C-x C-f c:\test.txt
>    => Use M-x make-directory RET RET to create the directory and its parents

I said nothing about "C-x C-f", and ffap doesn't go through it
(AFAIK).

Message #70 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: "Andrew W. Nosenko" <andrew.w.nosenko <at> gmail.com>
Cc: hochberg <at> dmhsoftware.com, Eli Zaretskii <eliz <at> gnu.org>,
	8439 <at> debbugs.gnu.org, Jari Aalto <jari.aalto <at> cante.net>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 14:01:04 +0300

> Even under Unix (where spaces inside filenames are very unusual)

They are unusual for Unix native files but not rare when files
created on Windows reach Unix file systems.  So this problem
is not unique to Windows (only the drive letter part of the problem
is Windows-specific).

> But, if there no way to make false positives amount small enough, I
> would to propose an idea for extension: find-file-at-region().  You
> just select a region of text, and Emacs tries to interprets it as
> default value for find-file().

ffap used to fetch the selected region until Stefan removed this feature
with the following comment:

             (let ((mark-active nil))
               ;; Don't use the region here, since it can be something
               ;; completely unwieldy.  If the user wants that, she could
               ;; use M-w before and then C-y.  --Stef

I have no problem with this change because ffap still can successfully fetch
filenames with spaces due to `ffap-completable' that works surprisingly well
in most cases (it incorrectly highlights the filename part until the first
space, but correctly fetches the complete filename with spaces).

Message #73 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 8439 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2)
Date: Sat, 20 Oct 2012 20:16:53 -0400

> Someone said in another discussion that Cygwin already does accept
> Windows-style file names.

I think that was a misunderstanding.  But Cygwin Emacs probably should
understand Windows-style file names to some extent (probably via
a file-name handler).


        Stefan

Message #76 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: jari <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 20 Oct 2012 20:18:15 -0400

>    - remove functions related to Cygwin and use
>      cygwin-convert-path-from-windows

Again, I don't think ffap should do this.  If you want to use Windows
file names in Cygwin Emacs, then make Cygwin Emacs understand those file
names and not just in ffap.


        Stefan "Haven't had time to look at the patch yet"

Message #79 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jari Aalto <jari.aalto <at> cante.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sun, 21 Oct 2012 11:07:53 +0300

2012-10-21 03:18 Stefan Monnier <monnier <at> iro.umontreal.ca>:
| >    - remove functions related to Cygwin and use
| >      cygwin-convert-path-from-windows
| 
| Again, I don't think ffap should do this.

Files under Cygwin Emacs are opened e.g. in $HOME
or //SHARE/ directories; those contain lines with windows style paths.

| If you want to use Windows file names in Cygwin Emacs, then make Cygwin
| Emacs understand those file names and not just in ffap.

I have no expertise on that area. I'm sure someone can continue to
generalize further.

Speaking from a user's perpective, it would be nice to have an improved
ffap in next release, as the common use is:

  * move point under file name
  * call ffap

   => it reads the file name, suitable for current environment (Cygwin).

Jari

Message #84 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: jari <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org, 6695 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 23 Nov 2019 14:31:47 +0100

jari <jari.aalto <at> cante.net> writes:

> In this patch (compared to previous v3):
>
>    - remove functions related to Cygwin and use cygwin-convert-path-from-windows
>
>    - obey END parameter in ffap-search-backward-file-end as originally
>      intended
>
>    - correct "path-separator" term into "dir-separator" everywhere

This was seven years ago, and not surprisingly, the patch doesn't apply
any more.

But I think the approach here seemed good -- going back in the buffer
and check whether you find something that matches a file on the file
system (if I understood the patch correctly).

Is there still interest in making this work, or has everybody given up
on using ffap with file names with spaces?

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

Message #87 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: jari <jari.aalto <at> cante.net>
Cc: 8439 <at> debbugs.gnu.org, 6695 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Fri, 14 Aug 2020 15:08:45 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> This was seven years ago, and not surprisingly, the patch doesn't apply
> any more.

I've respun the patch so that it now applies to Emacs 28, and the test
cases seem to kinda work?  The c: isn't included, but is that to be
expected?

I removed the Cygwin char translation stuff, because there was some
discussion about whether that was needed.

So what do people think?  Good or bad?  Does this work in any way
sensibly for people?

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 4a506207d5..6d40fa8c45 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1109,6 +1109,123 @@ ffap-string-at-point
   ;; Added at suggestion of RHOGEE (for ff-paths), 7/24/95.
   "Last string returned by the function `ffap-string-at-point'.")
 
+;; Test cases: (let ((ffap-file-name-with-spaces-flag t)) (ffap-string-at-point))
+;;
+;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program here.txt
+;; c:/Program Files/Open Text Evaluation Media/Open Text Exceed 14 x86/Program Files/Hummingbird/
+;; c:\Program Files\Open Text Evaluation Media\Open Text Exceed 14 x86\Program Files\Hummingbird\
+;; c:\Program Files\Freescale\CW for MPC55xx and MPC56xx 2.10\PowerPC_EABI_Tools\Command_Line_Tools\CLT_Usage_Notes.txt
+;; C:\temp\program.log on Windows or /var/log/program.log on Unix.
+
+(defvar ffap-file-name-with-spaces-flag (memq system-type '(ms-dos windows-nt))
+  "If non-nil, enable looking for paths with spaces in `ffap-string-at-point'.
+Enabled in W32 by default.")
+
+(defun ffap-search-backward-file-end (&optional dir-separator end)
+  "Search backward position point where file would probably end.
+Optional DIR-SEPARATOR defaults to \"/\". The search maximum is
+`line-end-position' or optional END point.
+
+Suppose the cursor is somewhere that might be near end of file,
+the guessing would position point before punctuation (like comma)
+after the file extension:
+
+  C:\temp\file.log, which contain ....
+  =============================== (before)
+  ---------------- (after)
+
+
+  C:\temp\file.log on Windows or /tmp/file.log on Unix
+  =============================== (before)
+  ---------------- (after)
+
+The strategy is to search backward until DIR-SEPARATOR which defaults to
+\"/\" and then take educated guesses.
+
+Move point and return point if an adjustment was done."
+  (unless dir-separator
+    (setq dir-separator "/"))
+  (let ((opoint (point))
+	point punct end whitespace-p)
+    (when (re-search-backward
+	   (regexp-quote dir-separator) (line-beginning-position) t)
+      ;; Move to the beginning of the match..
+      (forward-char 1)
+      ;; ... until typical punctuation.
+      (when (re-search-forward "\\([][<>()\"'`,.:;]\\)"
+			       (or end
+				   (line-end-position))
+			       t)
+	(setq end (match-end 0))
+	(setq punct (match-string 1))
+	(setq whitespace-p (looking-at "[ \t\r\n]\\|$"))
+	(goto-char end)
+	(cond
+	 ((and (string-equal punct ".")
+	       whitespace-p)            ;end of sentence
+	  (setq point (1- (point))))
+	 ((and (string-equal punct ".")
+	       (looking-at "[a-zA-Z0-9.]+")) ;possibly file extension
+	  (setq point (match-end 0)))
+	 (t
+	  (setq point (point)))))
+      (goto-char opoint)
+      (when point
+	(goto-char point)
+	point))))
+
+(defun ffap-search-forward-file-end (&optional dir-separator)
+  "Search DIR-SEPARATOR and position point at file's maximum ending.
+This includes spaces.
+Optional DIR-SEPARATOR defaults to \"/\".
+Call `ffap-search-backward-file-end' to refine the ending point."
+  (unless dir-separator
+    (setq dir-separator "/"))
+  (let* ((chars                         ;expected chars in file name
+	  (concat "[^][^<>()\"'`;,#*|"
+		  ;; exclude the opposite as we know the separator
+		  (if (string-equal dir-separator "/")
+		      "\\\\"
+		    "/")
+		  "\t\r\n]"))
+	 (re (concat
+	      chars "*"
+	      (if dir-separator
+		  (regexp-quote dir-separator)
+		"/")
+	      chars "*")))
+    (when (looking-at re)
+      (goto-char (match-end 0)))))
+
+(defun ffap-dir-separator-near-point ()
+  "Search backward and forward for closest slash or backlash in line.
+Return string slash or backslash. Point is moved to closest position."
+  (let ((point (point))
+	str pos)
+    (when (looking-at ".*?/")
+      (setq str "/"
+	    pos (match-end 0)))
+    (when (and (looking-at ".*?\\\\")
+               (or (null pos)
+	           (< (match-end 0) pos)))
+      (setq str "\\"
+	    pos (match-end 0)))
+    (goto-char point)
+    (when (and (re-search-backward "/" (line-beginning-position) t)
+               (or (null pos)
+	           (< (- point (point)) (- pos point))))
+      (setq str "/"
+	    pos (1+ (point)))) ;1+ to keep cursor at the end of char
+    (goto-char point)
+    (when (and (re-search-backward "\\\\" (line-beginning-position) t)
+               (or (null pos)
+		   (< (- point (point)) (- pos point))))
+      (setq str "\\"
+	    pos (1+ (point))))
+    (when pos
+      (goto-char pos))
+    str))
+
 (defun ffap-string-at-point (&optional mode)
   "Return a string of characters from around point.
 
@@ -1128,7 +1245,8 @@ ffap-string-at-point
 
 When the region is active and larger than `ffap-max-region-length',
 return an empty string, and set `ffap-string-at-point-region' to '(1 1)."
-  (let* ((args
+  (let* (dir-separator
+         (args
 	  (cdr
 	   (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
 	       (assq 'file ffap-string-at-point-mode-alist))))
@@ -1137,14 +1255,25 @@ ffap-string-at-point
          (beg (if region-selected
 		  (region-beginning)
 		(save-excursion
-		  (skip-chars-backward (car args))
-		  (skip-chars-forward (nth 1 args) pt)
+	          (if (and ffap-file-name-with-spaces-flag
+			   (memq mode '(nil file)))
+		      (when (setq dir-separator (ffap-dir-separator-near-point))
+		        (while (re-search-backward
+			        (regexp-quote dir-separator)
+			        (line-beginning-position) t)
+		          (goto-char (match-beginning 0))))
+		    (skip-chars-backward (car args))
+		    (skip-chars-forward (nth 1 args) pt))
 		  (point))))
          (end (if region-selected
 		  (region-end)
 		(save-excursion
 		  (skip-chars-forward (car args))
 		  (skip-chars-backward (nth 2 args) pt)
+	          (when (and ffap-file-name-with-spaces-flag
+			     (memq mode '(nil file)))
+		    (ffap-search-forward-file-end dir-separator)
+		    (ffap-search-backward-file-end dir-separator))
 		  (point))))
          (region-len (- (max beg end) (min beg end))))
 


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

Message #90 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 8439 <at> debbugs.gnu.org, 6695 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 15 Aug 2020 12:07:52 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: 8439 <at> debbugs.gnu.org,  6695 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
> Date: Fri, 14 Aug 2020 15:08:45 +0200
> 
> I've respun the patch so that it now applies to Emacs 28, and the test
> cases seem to kinda work?

Thanks.

> The c: isn't included, but is that to be expected?

Is it?  I thought the intent was to include the full file name, which
means the drive letter should be included.  Otherwise the file will
not be found.

> So what do people think?  Good or bad?  Does this work in any way
> sensibly for people?

Since the heuristic only covers some use cases, I think we should have
it off by default, and we should document its potential pitfalls in
the doc string.  With those qualifications, I'm okay with adding this
optional feature.

Message #93 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 8439 <at> debbugs.gnu.org, 6695 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: Re: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 15 Aug 2020 12:13:04 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> The c: isn't included, but is that to be expected?
>
> Is it?  I thought the intent was to include the full file name, which
> means the drive letter should be included.  Otherwise the file will
> not be found.

I don't have a Windows system to test with...  it's possible that the
functions return the file name including the c: on a Windows machine?

>> So what do people think?  Good or bad?  Does this work in any way
>> sensibly for people?
>
> Since the heuristic only covers some use cases, I think we should have
> it off by default, and we should document its potential pitfalls in
> the doc string.  With those qualifications, I'm okay with adding this
> optional feature.

OK, I've added some tests and pushed it to the trunk.

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

Message #100 received at 8439 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 8439 <at> debbugs.gnu.org, 6695 <at> debbugs.gnu.org, jari.aalto <at> cante.net
Subject: RE: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v4)
Date: Sat, 15 Aug 2020 12:33:48 -0700 (PDT)

> > The c: isn't included, but is that to be expected?
> 
> Is it?  I thought the intent was to include the full file name, which
> means the drive letter should be included.  Otherwise the file will
> not be found.
> 
> > So what do people think?  Good or bad?  Does this work in any way
> > sensibly for people?
> 
> Since the heuristic only covers some use cases, I think we should have
> it off by default, and we should document its potential pitfalls in
> the doc string.  With those qualifications, I'm okay with adding this
> optional feature.

FWIW: As the submitter of the original bug report
(#6695), I'd say this shouldn't be marked "fixed"
if the drive letter part (e.g. c:/) isn't included
on MS Windows.

It sounds like it represents some progress, but
it sounds like it's not fixed yet.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.