GNU bug report logs - #6695
24.0.50; thing-at-point-url-at-point and ffap-guesser problem

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 6695 in the body.
You can then email your comments to 6695 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: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.0.50; thing-at-point-url-at-point and ffap-guesser problem
Date: Wed, 21 Jul 2010 11:16:25 -0700

emacs -Q
 
Load thingatpt.el and ffap.el.  Use this test line of text:
 
;; c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf
 
1. Put the cursor on the g of Settings.
   M-: (thing-at-point-url-at-point) => "http://Settings/foobar/My"
 
2. Put the cursor on the S of MyStuff.
   M-: (thing-at-point-url-at-point) =>
   "http://Documents/MyStuff/foo.pdf"
 
Neither of those is remotely correct.
 
3. Put the cursor on the g of Settings.
   M-: (ffap-guesser) => nil
 
   Same thing with cursor *anywhere* on the absolute file name, except:
   If the cursor is on any character in this string: "c:/Documents" then
   (ffap-guesser) returns "c:/Documents and Settings/" (which is also
   wrong).
 
What should happen:
 
`ffap-guesser' should return
"c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf"
 
`thing-at-point-url-at-point should return
"http://c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf"
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-07-19 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags -Ic:/xpm/include'
 

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

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 6695 <at> debbugs.gnu.org
Subject: Re: 24.0.50; thing-at-point-url-at-point and ffap-guesser problem
Date: Thu, 14 Jul 2011 15:25:09 +0200

"Drew Adams" <drew.adams <at> oracle.com> writes:

> Load thingatpt.el and ffap.el.  Use this test line of text:
>
> ;; c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf
>
> 1. Put the cursor on the g of Settings.
>    M-: (thing-at-point-url-at-point) => "http://Settings/foobar/My"
>
> 2. Put the cursor on the S of MyStuff.
>    M-: (thing-at-point-url-at-point) =>
>    "http://Documents/MyStuff/foo.pdf"

These both return nil for me in Emacs 24.

> Neither of those is remotely correct.
>
> 3. Put the cursor on the g of Settings.
>    M-: (ffap-guesser) => nil

[...]

> What should happen:
>
> `ffap-guesser' should return
> "c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf"
>
> `thing-at-point-url-at-point should return
> "http://c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf"

I don't really see how guessing that these things are file names is
feasible.

Unless one adds special matches for Windows where [letter]:/ matches
stuff until the end of the line or something...

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

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

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Lars Magne Ingebrigtsen'" <larsi <at> gnus.org>
Cc: 6695 <at> debbugs.gnu.org
Subject: RE: 24.0.50; thing-at-point-url-at-point and ffap-guesser problem
Date: Thu, 14 Jul 2011 09:29:19 -0700

> These both return nil for me in Emacs 24.

Yes.  So it is still not fixed, but is broken in another way.

Well, to be fair, punting and returning nil is not incorrect in the sense that
it gives the wrong URL.  It is incorrect in that it does not give the (correct)
URL at all.  It says, in effect, there is no URL at point, which is wrong.

> > Neither of those is remotely correct.
> >
> > 3. Put the cursor on the g of Settings.
> >    M-: (ffap-guesser) => nil

In the case of `ffap-guesser' it is perhaps too strong to say that a nil value
indicates that there is no URL at point (as in the `thing-at-point' case).  It
is only claiming to "guess", whereas `thing-at-point' returning nil claims that
there is no URL at point, and programs should be able to depend on that.

> > What should happen:
> >
> > `ffap-guesser' should return
> > "c:/Documents and Settings/foobar/My Documents/MyStuff/foo.pdf"
> >
> > `thing-at-point-url-at-point should return
> > "http://c:/Documents and Settings/foobar/My 
> Documents/MyStuff/foo.pdf"
> 
> I don't really see how guessing that these things are file names is
> feasible.

Why not?  That's their job.

> Unless one adds special matches for Windows where [letter]:/ matches
> stuff until the end of the line or something...

Maybe.  Dunno.  It would be good for someone knowledgable in thingatpt.el and
ffap.el take a look and see how these cases can be improved.

In the case of ffap.el, I guess you could call this an enhancement request,
since returning `nil' is just giving up and saying it has no "guess".  In the
case of thingatpt.el, this is a bug: it claims incorrectly that there is no URL
at point.

Message #18 received at 6695 <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 #21 received at 6695 <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 #24 received at 6695 <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 #27 received at 6695 <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 #34 received at 6695 <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.