From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Kaushal Modi Newsgroups: gmane.emacs.bugs Subject: bug#24057: 25.1.50; ffap interprets comments beginning with "//" as file path Date: Tue, 26 Jul 2016 15:11:45 +0000 Message-ID: References: <83lh0sx0yf.fsf@gnu.org> <83shv0v716.fsf@gnu.org> <83poq4v4jk.fsf@gnu.org> <83wpkbt92i.fsf@gnu.org> <83mvl5tzv7.fsf@gnu.org> <83invttyv7.fsf@gnu.org> <83eg6gtqaq.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=94eb2c12f1d64776ad05388b52c2 X-Trace: ger.gmane.org 1469546011 2593 80.91.229.3 (26 Jul 2016 15:13:31 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 26 Jul 2016 15:13:31 +0000 (UTC) Cc: 24057@debbugs.gnu.org, npostavs@users.sourceforge.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Jul 26 17:13:22 2016 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 1bS42r-0006Ns-V4 for geb-bug-gnu-emacs@m.gmane.org; Tue, 26 Jul 2016 17:13:22 +0200 Original-Received: from localhost ([::1]:40454 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS42o-0006gu-4d for geb-bug-gnu-emacs@m.gmane.org; Tue, 26 Jul 2016 11:13:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS42d-0006c7-Py for bug-gnu-emacs@gnu.org; Tue, 26 Jul 2016 11:13:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bS42X-0001LA-Qo for bug-gnu-emacs@gnu.org; Tue, 26 Jul 2016 11:13:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:54164) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS42X-0001L6-NO for bug-gnu-emacs@gnu.org; Tue, 26 Jul 2016 11:13:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bS42X-0007rz-J6 for bug-gnu-emacs@gnu.org; Tue, 26 Jul 2016 11:13:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Kaushal Modi Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 26 Jul 2016 15:13:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24057 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 24057-submit@debbugs.gnu.org id=B24057.146954593230195 (code B ref 24057); Tue, 26 Jul 2016 15:13:01 +0000 Original-Received: (at 24057) by debbugs.gnu.org; 26 Jul 2016 15:12:12 +0000 Original-Received: from localhost ([127.0.0.1]:38268 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bS41h-0007qt-1b for submit@debbugs.gnu.org; Tue, 26 Jul 2016 11:12:12 -0400 Original-Received: from mail-it0-f41.google.com ([209.85.214.41]:34929) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bS41b-0007q8-VE for 24057@debbugs.gnu.org; Tue, 26 Jul 2016 11:12:07 -0400 Original-Received: by mail-it0-f41.google.com with SMTP id u186so115911720ita.0 for <24057@debbugs.gnu.org>; Tue, 26 Jul 2016 08:12:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=N8TxUvGOlPJ7EefyST/uEZ22csysGzZu93sWqutSkMs=; b=oCgDk9kyvTkMLplBbrt7upSvvj6GYTea+JERIX/zK1G2fzkyj7iPW21JvQ6yAyq/Ue jG1BH1FY4r3gMlUvxjXh2Lg4ex8AKd1+nuVJGtGQtzsKczUbFc7IeVqhFukXMyPVftlt 865ykfJGNjhEjBXXziJBfiVuoP2w6DQMyGi7a6t1PKJPCbWphcDA0iqd+h9pg72+QhmZ rfXcTR1p+EEfq8gVVbcfyPnyt5Y+SDkeOyLSDOjWKM6uuzivXv+vbkT9oDIqYwiS7zbS vVRpMsKeCfegbA3nKkU+CY7mGrkLDmEtaNaZ0mWEEGwkT08poa0lfNhGFANOm4IJ7HO0 +9Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=N8TxUvGOlPJ7EefyST/uEZ22csysGzZu93sWqutSkMs=; b=dWAnargwoOW0cggKNChKZ++qeQWQm7s5on2VvzxSjVt57NqiaFUZwO7oQyuJsb31pn thMMygV/p/8sQKLozaWg42+WfHBP8fiOEtUyrUJPauDvM//et56nC0js3GgSaylUVqCW /d+C+vrmTaUFqwUNxhwgmRzaWd1r9wr4sZy4G99X0Ln4Iw1tcbe5DX5VkXZ5TDi6otIm NKOO98nFH59gEAVileztdPwodSru6TZuE3jQLLF8IDcBi5KmBdgDsz/n2/cH4ATPOuKv syxpYz/sUpMzJxlFtFTQo+XkiIAueh+d66Zsup+1Sbebncn8FO8sUfxiYcjyOCsT0V4j 3bWg== X-Gm-Message-State: AEkoouvqh/fmoefE49hVJtmXl392QhUqWwjBojC1dB02HUVc8Vaw2UFVLrAljIR6aDsk2Y+n+p2zI5sZOLe0Ow== X-Received: by 10.202.8.210 with SMTP id 201mr11540338oii.107.1469545915575; Tue, 26 Jul 2016 08:11:55 -0700 (PDT) In-Reply-To: <83eg6gtqaq.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.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" Xref: news.gmane.org gmane.emacs.bugs:121558 Archived-At: --94eb2c12f1d64776ad05388b52c2 Content-Type: text/plain; charset=UTF-8 On Tue, Jul 26, 2016 at 10:42 AM Eli Zaretskii wrote: > > Here a git formatted patch .. it's an optimized, generic version of the > > snippet in my previous email. It's now "/" agnostic. > > Thanks. Most of the patch looks like whitespace changes? > I selected the whole defun and did auto-ident (C-M-\). The submitted patch was a result of that. I thought it's a good idea to update the indentation of the whole function. May be the lisp-indent-function changed since this function was updated last time? In any case, here is the patch ignoring whitespace changes. Note of caution that the indentation will look messed up when you merge this patch: ===== diff --git a/lisp/ffap.el b/lisp/ffap.el index 7013e6e..d1bca04 100644 --- a/lisp/ffap.el +++ b/lisp/ffap.el @@ -1097,33 +1097,73 @@ ffap-string-at-point (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 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 the variable `ffap-string-at-point' and the variable -`ffap-string-at-point-region'." + +If the point is in a comment, ensure that the returned string does not contain +the comment start characters (especially for major modes that have '//' as +comment start characters). + +Sets variables `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)))) + (region-selected (use-region-p)) (pt (point)) - (beg (if (use-region-p) + (beg (if region-selected (region-beginning) (save-excursion (skip-chars-backward (car args)) (skip-chars-forward (nth 1 args) pt) (point)))) - (end (if (use-region-p) + (end (if region-selected (region-end) (save-excursion (skip-chars-forward (car args)) (skip-chars-backward (nth 2 args) pt) - (point))))) + (point)))) + (beg-new beg)) + ;; (message "ffap-string-at-point dbg: beg = %d end = %d" beg end) + ;; If the initial characters of the to-be-returned string are the + ;; current major mode's comment starter characters, *and* are not part + ;; of a comment, remove those from the returned string (Bug#24057). + ;; Example comments in `c-mode' (which considers lines beginning with + ;; "//" as comments): + ;; //tmp - This is a comment. It does not contain any path reference. + ;; ///tmp - This is a comment. The "/tmp" portion in that is a path. + ;; ////tmp - This is a comment. The "//tmp" portion in that is a path. + (when (and + ;; Proceed if no region is selected by the user. + (null region-selected) + ;; Check if END character is part of a comment. + (save-excursion + (goto-char end) + (nth 4 (syntax-ppss)))) + (save-excursion + ;; Increment BEG till point at BEG is in a comment too. + ;; (nth 4 (syntax-ppss)) will be nil for comment start characters + ;; (for example, for the "//" characters in `c-mode' line comments). + (setq beg (catch 'break + (while (< beg-new end) + (goto-char beg-new) + (if (nth 4 (syntax-ppss)) ; in a comment + (throw 'break beg-new) + (setq beg-new (1+ beg-new)))) + end)))) ; Set BEG to END if no throw happens + ;; (message "ffap-string-at-point dbg: beg = %d beg-new = %d" beg beg-new) (setq ffap-string-at-point (buffer-substring-no-properties (setcar ffap-string-at-point-region beg) - (setcar (cdr ffap-string-at-point-region) end))))) + (setcar (cdr ffap-string-at-point-region) end))) + ;; (message "ffap-string-at-point dbg: ffap-string-at-point = %S" + ;; ffap-string-at-point) + ffap-string-at-point)) (defun ffap-string-around () ;; Sometimes useful to decide how to treat a string. ===== > + (setq beg (catch 'break > > + (while (< beg-new end) > > + (goto-char beg-new) > > + (if (nth 4 (syntax-ppss)) ; in a comment > > + (throw 'break beg-new) > > + (setq beg-new (1+ beg-new)))) > > + end)))) ; Set BEG to END if no throw happens > > Is there any problem with a more conventional while loop that stops > when syntax-ppss has its 5th element non-nil? Why did you need to use > throw and catch here? > Throw and catch just feels more intuitive to me. This construct also sets beg to end if no throw happens. It's just a difference of style; feel free to refactor it. -- Kaushal Modi --94eb2c12f1d64776ad05388b52c2 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Tue, Jul 26= , 2016 at 10:42 AM Eli Zaretskii <eliz@g= nu.org> wrote:
> Here a g= it formatted patch .. it's an optimized, generic version of the
> snippet in my previous email. It's now "/" agnostic.

Thanks.=C2=A0 Most of the patch looks like whitespace changes?

I selected the whole defun and did auto-ident (C-M-= \). The submitted patch was a result of that. I thought it's a good ide= a to update the indentation of the whole function. May be the lisp-indent-f= unction changed since this function was updated last time?

In any case, here is the patch ignoring whitespace changes. Note o= f caution that the indentation will look messed up when you merge this patc= h:

=3D=3D=3D=3D=3D

diff --git a/lisp/ffap.el b/lisp/ffap= .el
index 7013e6e..d1bca04 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1097,33 +1097,73 @@ ffap-string-at= -point
=C2=A0
=C2=A0(defun ffap-string-at-point (&o= ptional mode)
=C2=A0 =C2=A0"Return a string of characters fr= om around point.
+
=C2=A0MODE (defaults to value of `ma= jor-mode') is a symbol used to look up
=C2=A0string syntax pa= rameters in `ffap-string-at-point-mode-alist'.
+
= =C2=A0If MODE is not found, we use `file' instead of MODE.
+<= /div>
=C2=A0If the region is active,return a string from the region.
-Sets the variable `ffap-string-at-point' and the variable
-`ffap-string-at-point-region'."
+
+If t= he point is in a comment, ensure that the returned string does not contain<= /div>
+the comment start characters (especially for major modes that ha= ve '//' as
+comment start characters).
+
<= div>+Sets variables `ffap-string-at-point' and `ffap-string-at-point-re= gion'. "
=C2=A0 =C2=A0(let* ((args
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cdr
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 (or (assq (or mode major-mode) ffap-string-at-point-mode-alis= t)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (assq = 'file ffap-string-at-point-mode-alist))))
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0 (region-selected (use-region-p))
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 (pt (point))
- (beg (if (use-region-p)
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0 (beg (if region-selected
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(region-beginning)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(save-e= xcursion
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0(skip-chars-backward (car args))
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(skip-chars-forward (nt= h 1 args) pt)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0(point))))
- (end (if (use-region-p)
+ =C2=A0= =C2=A0 =C2=A0 =C2=A0 (end (if region-selected
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(region-end)
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(save-excursi= on
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0(skip-chars-forward (car args))
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(skip-chars-backward (nth 2 ar= gs) pt)
- =C2=A0(point)))))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0(point))))
+ =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (beg-new beg))
+ =C2=A0 =C2=A0;; (message "ffap-string-a= t-point dbg: beg =3D %d end =3D %d" beg end)
+ =C2=A0 =C2=A0= ;; If the initial characters of the to-be-returned string are the
+ =C2=A0 =C2=A0;; current major mode's comment starter characters, *an= d* are not part
+ =C2=A0 =C2=A0;; of a comment, remove those from= the returned string (Bug#24057).
+ =C2=A0 =C2=A0;; Example comme= nts in `c-mode' (which considers lines beginning with
+ =C2= =A0 =C2=A0;; "//" as comments):
+ =C2=A0 =C2=A0;; =C2= =A0//tmp - This is a comment. It does not contain any path reference.
=
+ =C2=A0 =C2=A0;; =C2=A0///tmp - This is a comment. The "/tmp&quo= t; portion in that is a path.
+ =C2=A0 =C2=A0;; =C2=A0////tmp - T= his is a comment. The "//tmp" portion in that is a path.
+ =C2=A0 =C2=A0(when (and
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = ;; Proceed if no region is selected by the user.
+ =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 (null region-selected)
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 ;; Check if END character is part of a comment.
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (save-excursion
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (goto-char end)
+ =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (nth 4 (syntax-ppss))))
+ =C2=A0 = =C2=A0 =C2=A0(save-excursion
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0;; Incr= ement BEG till point at BEG is in a comment too.
+ =C2=A0 =C2=A0 = =C2=A0 =C2=A0;; (nth 4 (syntax-ppss)) will be nil for comment start charact= ers
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0;; (for example, for the "/= /" characters in `c-mode' line comments).
+ =C2=A0 =C2= =A0 =C2=A0 =C2=A0(setq beg (catch 'break
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(while (< beg-new en= d)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(goto-char beg-new)
+ =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (nth 4 (syntax-ppss= )) ; in a comment
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(throw 'break beg-new)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(setq beg-new (1+ beg-new))))
+ =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0end)))) ; Set BEG to= END if no throw happens
+ =C2=A0 =C2=A0;; (message "ffap-st= ring-at-point dbg: beg =3D %d beg-new =3D %d" beg beg-new)
= =C2=A0 =C2=A0 =C2=A0(setq ffap-string-at-point
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0(buffer-substring-no-properties
=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (setcar ffap-string-at-point-region beg)=
- =C2=A0 (setcar (cdr ffap-string-at-point-region) end)))))
+ = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (setcar (cdr ffap-string-at-point-region= ) end)))
+ =C2=A0 =C2=A0;; (message "ffap-string-at-point db= g: ffap-string-at-point =3D %S"
+ =C2=A0 =C2=A0;; =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0ffap-string-at-point)
+ =C2=A0 =C2=A0ffap= -string-at-point))
=C2=A0
=C2=A0(defun ffap-string-arou= nd ()
=C2=A0 =C2=A0;; Sometimes useful to decide how to treat a s= tring.
= =3D=3D=3D=3D=3D=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (setq beg (catch 'break
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (while (< beg-new end)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 (goto-char beg-new)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 (if (nth 4 (syntax-ppss)) ; in a comment
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 (throw 'break beg-new)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 (setq beg-new (1+ beg-new))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= end)))) ; Set BEG to END if no throw happens

Is there any problem with a more conventional while loop that stops
when syntax-ppss has its 5th element non-nil?=C2=A0 Why did you need to use=
throw and catch here?

Throw and catch j= ust feels more intuitive to me. This construct also sets beg to end if no t= hrow happens.
It's just a difference of style; feel free to r= efactor it.
--

Kaushal Modi

--94eb2c12f1d64776ad05388b52c2--