From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Daniel Colascione Newsgroups: gmane.emacs.bugs Subject: bug#8439: [PATCH] ffap.el -- detect paths with spaces (v2) Date: Fri, 19 Oct 2012 18:49:10 -0700 Message-ID: <50820316.1030402@dancol.org> References: <87pqoyaxu0.fsf@blue.sea.net> <87zk3i7tbu.fsf@picasso.cante.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigB3AC3761F5B2FBD2A4D92E85" X-Trace: ger.gmane.org 1350697775 5044 80.91.229.3 (20 Oct 2012 01:49:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 20 Oct 2012 01:49:35 +0000 (UTC) Cc: 8439@debbugs.gnu.org, Jari Aalto To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Oct 20 03:49:42 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TPOC4-0003U0-Rs for geb-bug-gnu-emacs@m.gmane.org; Sat, 20 Oct 2012 03:49:41 +0200 Original-Received: from localhost ([::1]:51522 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TPOBx-00022a-MV for geb-bug-gnu-emacs@m.gmane.org; Fri, 19 Oct 2012 21:49:33 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:43056) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TPOBu-00022V-RW for bug-gnu-emacs@gnu.org; Fri, 19 Oct 2012 21:49:32 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TPOBt-00077r-GX for bug-gnu-emacs@gnu.org; Fri, 19 Oct 2012 21:49:30 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:42232) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TPOBt-00077n-D8 for bug-gnu-emacs@gnu.org; Fri, 19 Oct 2012 21:49:29 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1TPODN-0006yv-KY for bug-gnu-emacs@gnu.org; Fri, 19 Oct 2012 21:51:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Colascione Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 20 Oct 2012 01:51:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8439 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 8439-submit@debbugs.gnu.org id=B8439.135069785526823 (code B ref 8439); Sat, 20 Oct 2012 01:51:01 +0000 Original-Received: (at 8439) by debbugs.gnu.org; 20 Oct 2012 01:50:55 +0000 Original-Received: from localhost ([127.0.0.1]:52483 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TPODH-0006ya-AF for submit@debbugs.gnu.org; Fri, 19 Oct 2012 21:50:55 -0400 Original-Received: from dancol.org ([96.126.100.184]:43890) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TPODC-0006yP-OE for 8439@debbugs.gnu.org; Fri, 19 Oct 2012 21:50:53 -0400 Original-Received: from c-76-22-66-162.hsd1.wa.comcast.net ([76.22.66.162] helo=[0.0.0.0]) by dancol.org with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.72) (envelope-from ) id 1TPOBf-0003oG-O3; Fri, 19 Oct 2012 18:49:15 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20121010 Thunderbird/16.0.1 In-Reply-To: X-Enigmail-Version: 1.4.5 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:65772 Archived-At: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigB3AC3761F5B2FBD2A4D92E85 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 10/19/2012 6:44 PM, Stefan Monnier wrote: > A few comments about your patch below. >=20 >=20 > Stefan >=20 >=20 >> + (let* ((cygwin-p (string-match "cygwin" (emacs-version))) >=20 > Test system-type instead. >=20 >> + (if (and ffap-paths-with-spaces >> + (memq mode '(nil file))) >> + (if (string-match "^[ \t]*$" >> + (buffer-substring (line-beginning-position) >> + (point))) >=20 > Better match the buffer (e.g. with forward-line + looking-at, or in thi= s > case with skip-chars-backward + bolp) than extract a part of the buffer= > and then apply string-match to it. >=20 >> + ;; Nothing interesting before point. Move to the first chara= cter >=20 > Please terminate your comments with proper punctuation. >=20 >> + (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))))) >=20 > This code seems to assume Windows-style file names, whereas none of the= > var names nor comments mention anything about this assumption. >=20 > 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 mak= e > this reasoning explicit in a comment. >=20 >> + (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[:s= pace:]]") >> + (setq space-p (match-end 0)))) >=20 > 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'. >=20 >> + (setq end (point)) >> + (if (and space-p >> + (> space-p end) >> + (memq mode '(file nil))) >> + (setq end space-p)) >=20 > 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. >=20 >> + ;; 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= ))))) >=20 > 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 mappin= g. (Not everyone's Cygwin drive prefix is "/cygdrive".) --------------enigB3AC3761F5B2FBD2A4D92E85 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Cygwin) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCCAxkACgkQ17c2LVA10VveRQCfU+8NaVzN0583Wol++U72QDqO yKsAn1lOaKATMNvDjKKYHBsutsoWTQ3l =1FQT -----END PGP SIGNATURE----- --------------enigB3AC3761F5B2FBD2A4D92E85--