GNU bug report logs - #42168
26.1; cperl-mode: Bad interpretation of $a++ / $b

Previous Next

Package: emacs;

Reported by: Harald Jörg <haj <at> posteo.de>

Date: Thu, 2 Jul 2020 19:37:02 UTC

Severity: minor

Tags: confirmed, patch

Merged with 36946

Found in versions 26.1, 26.2, 28.0.50

Fixed in version 28.1

Done: Stefan Kangas <stefankangas <at> gmail.com>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 02 Jul 2020 19:37:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Harald Jörg <haj <at> posteo.de>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 02 Jul 2020 19:37:02 GMT) Full text and rfc822 format available.

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

From: Harald Jörg <haj <at> posteo.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Thu, 2 Jul 2020 20:19:21 +0200
[Message part 1 (text/plain, inline)]
How to reproduce:
In a buffer, activate cperl-mode (M-x cperl-mode), then enter:

$a++ / $b;

This is supposed to be a division.  However, cperl-mode interprets the
slash as the start of a regular expression.  Accordingly, the following
text is displayed with the wrong face, and also cperl-mode might
complain that it can't find the end of the regular expression.

Emacs does not crash.

Attached: A patch which fixes the issue for $a++ and $a--, and also
includes a test to verify that the very similar text "$a+ / $b" is still
correctly interpreted as a regular expression.

Disclaimer: I'm rather new to elisp and even newer to ERT.

Cheers,
haj

[postincrement.diff (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Sat, 11 Jul 2020 09:55:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Harald Jörg <haj <at> posteo.de>
Cc: 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Sat, 11 Jul 2020 12:53:49 +0300
> From: Harald Jörg <haj <at> posteo.de>
> Date: Thu, 2 Jul 2020 20:19:21 +0200
> 
> How to reproduce:
> In a buffer, activate cperl-mode (M-x cperl-mode), then enter:
> 
> $a++ / $b;
> 
> This is supposed to be a division.  However, cperl-mode interprets the
> slash as the start of a regular expression.  Accordingly, the following
> text is displayed with the wrong face, and also cperl-mode might
> complain that it can't find the end of the regular expression.
> 
> Emacs does not crash.
> 
> Attached: A patch which fixes the issue for $a++ and $a--, and also
> includes a test to verify that the very similar text "$a+ / $b" is still
> correctly interpreted as a regular expression.

Thanks.

To accept a substantial contribution such as this one, we will need
you to assign the copyright of your code to the FSF.  If you are okay
with starting your legal paperwork, I will send you the form to sign
and email.

Thank you for your interest in Emacs.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 13 Aug 2020 00:41:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 42168 <at> debbugs.gnu.org, Harald Jörg <haj <at> posteo.de>
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Wed, 12 Aug 2020 17:40:19 -0700
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Harald Jörg <haj <at> posteo.de>
>> Date: Thu, 2 Jul 2020 20:19:21 +0200
>>
>> How to reproduce:
>> In a buffer, activate cperl-mode (M-x cperl-mode), then enter:
>>
>> $a++ / $b;
>>
>> This is supposed to be a division.  However, cperl-mode interprets the
>> slash as the start of a regular expression.  Accordingly, the following
>> text is displayed with the wrong face, and also cperl-mode might
>> complain that it can't find the end of the regular expression.
>>
>> Emacs does not crash.
>>
>> Attached: A patch which fixes the issue for $a++ and $a--, and also
>> includes a test to verify that the very similar text "$a+ / $b" is still
>> correctly interpreted as a regular expression.
>
> Thanks.
>
> To accept a substantial contribution such as this one, we will need
> you to assign the copyright of your code to the FSF.  If you are okay
> with starting your legal paperwork, I will send you the form to sign
> and email.
>
> Thank you for your interest in Emacs.

Harald, would you be willing to sign the copyright papers?  Please find
out more about the reason for this here:

    https://www.gnu.org/licenses/why-assign.html

Best regards,
Stefan Kangas




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 13 Aug 2020 07:31:02 GMT) Full text and rfc822 format available.

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

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefan <at> marxist.se>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Thu, 13 Aug 2020 09:30:41 +0200
On 8/13/20 2:40 AM, Stefan Kangas wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>>> From: Harald Jörg <haj <at> posteo.de>
>>> [...]
>> Thanks.
>>
>> To accept a substantial contribution such as this one, we will need
>> you to assign the copyright of your code to the FSF.  If you are okay
>> with starting your legal paperwork, I will send you the form to sign
>> and email.
>>
>> Thank you for your interest in Emacs.
> 
> Harald, would you be willing to sign the copyright papers?  Please find
> out more about the reason for this here:
> 
>     https://www.gnu.org/licenses/why-assign.html

I forgot to inform anyone, but this process has already been completed!
According to Lars Ingebrigtsen my name is already on "the list", so the
legal stuff should be out of the way.
-- 
Cheers,
haj





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 13 Aug 2020 07:56:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Harald Jörg <haj <at> posteo.de>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Thu, 13 Aug 2020 00:55:30 -0700
Harald Jörg <haj <at> posteo.de> writes:

> I forgot to inform anyone, but this process has already been completed!
> According to Lars Ingebrigtsen my name is already on "the list", so the
> legal stuff should be out of the way.

Excellent, welcome to Emacs!  I will review your patch now.

Best regards,
Stefan Kangas




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 13 Aug 2020 08:19:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>, 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Thu, 13 Aug 2020 01:17:59 -0700
Hi Harald,

Please see my comments on your patch below.

Harald Jörg <haj <at> posteo.de> writes:

> diff --git a/test/lisp/progmodes/cperl-mode/cperl-fontification-tests.el b/test/lisp/progmodes/cperl-mode/cperl-fontification-tests.el
> new file mode 100644
> index 0000000000..4148db036c
> --- /dev/null
> +++ b/test/lisp/progmodes/cperl-mode/cperl-fontification-tests.el

Great initiative to also write tests for this bug.

For now, I think you should simply place the file directly at:

lisp/progmodes/cperl-mode-tests.el

We could always break it up into several files later when it gets bigger
and there is a need to do so.

> @@ -0,0 +1,55 @@
> +;;; cperl-fontification-tests.el --- Test fontification in cperl-mode -*- lexical-binding: t -*-
> +
> +;; Copyright (C) 2020-2020 ...to be decided ...

All files in GNU Emacs should be following this template:

;; Copyright (C) 2020 Free Software Foundation, Inc.

(Note that you only need a range if there's more than one year.)

> +;; Author: Harald Jörg <haj <at> posteo.de>
> +;; Maintainer: Harald Jörg
> +;; Keywords:       internal
> +;; Human-Keywords: internal

Nit: I think we can skip Human-Keywords here.

> +;; Homepage: https://github.com/HaraldJoerg/cperl-mode
> +
> +;;; Commentary:
> +
> +;; This is a collection of Tests for the fontification of CPerl-mode.

Typo: "tests" shouldn't have a capital letter.

> +;; The primary target is to verify that the refactoring we're doing
> +;; right now (May 2020 - ...) doesn't change any behavior, or does the
> +;; right thing in cases where new fontification rules are enabled.

I think these three lines here could be removed, maybe?  It seems to me
that they are describing the purpose of all tests, namely to stop
regressions from happening.

> +;; Run these tests interactively:
> +;; (ert-run-tests-interactively '(tag :fontification))

Nit: Missing this header here:

;;; Code:

> +
> +(defun cperl-test-face (text regexp)
> +  "Returns the face of the first character matched by REGEXP in TEXT."
> +  (interactive)
> +  (with-temp-buffer
> +    (let ((cperl-hairy nil)
> +	  (cperl-font-lock nil)) ;; Does this matter?

Does it matter?  Not sure, what happens if you remove it?  :-)

> +      (insert text)
> +      (cperl-mode)
> +      (font-lock-fontify-buffer)
> +      (goto-char (point-min))
> +      (re-search-forward regexp)
> +      (message "%s" (match-string 0))
> +      (get-text-property (match-beginning 0) 'face))))

It is good practice to remove calls to 'message' in the tests, since
they mostly make the tests more noisy.  If it's really useful during
development of tests, you could comment it out or make it dependent upon
a new variable like

> +(ert-deftest jrockway-issue-45 ()

Is probably better named as: cperl-mode-test-bug-42168 to refer back to
our own bug in the name.  We already have the link to jrockway in the
doc string, which is useful if one needs to dig deeper.

> +  "Verify that '/' is a division after ++ or --, not a regexp.
> +Reported in https://github.com/jrockway/cperl-mode/issues/45.
> +If seen as regular expression, then the slash is displayed using
> +font-lock-constant-face.  If seen as a division, then it doesn't
> +have a face property."
> +  :tags '(:fontification)
> +  ;; The next two Perl expressions have divisions.  Perl "punctuation"
> +  ;; operators don't get a face.  The comment at the end of line
> +  ;; prevents cperl-mode from tripping over "End of ‘/ ... /’
> +  ;; string/RE not found" if it gets it wrong

Do we need the comment at the end of the below code fragments with your
patch as well?  If not, couldn't we just remove them?  I think that
would make the intention a little bit clearer, maybe.

> +  (let ((code "{ $a++ / $b } # /"))
> +    (should (equal (cperl-test-face code "/" ) nil)))
> +  (let ((code "{ $a-- / $b } # /"))
> +    (should (equal (cperl-test-face code "/" ) nil)))
> +  ;; The next two Perl expressions have regular expressions.  The
> +  ;; delimiter of a RE is fontified with font-lock-constant-face.
> +  (let ((code "{ $a+ / $b } # /"))
> +    (should (equal (cperl-test-face code "/" ) font-lock-constant-face)))
> +  (let ((code "{ $a- / $b } # /"))
> +    (should (equal (cperl-test-face code "/" ) font-lock-constant-face))))

The rest looks good to me.

Best regards,
Stefan Kangas




Added tag(s) confirmed and patch. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Thu, 13 Aug 2020 08:21:01 GMT) Full text and rfc822 format available.

bug Marked as found in versions 28.0.50. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Thu, 13 Aug 2020 08:21:01 GMT) Full text and rfc822 format available.

Removed tag(s) patch. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Thu, 13 Aug 2020 16:25:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Thu, 13 Aug 2020 20:41:01 GMT) Full text and rfc822 format available.

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

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>, 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Thu, 13 Aug 2020 22:40:37 +0200
[Message part 1 (text/plain, inline)]
Hello Stefan,

Thank you for your thorough review.  I'm rather new to Emacs Lisp, and
also to the Emacs development workflows, so this is highly
appreciated.

Here I've attached a new version of the patch.
 - Copyright notice is now as it should be (now that the paperwork
   permits it)
 - All the stuff you suggested to be removed has been removed
 - The test has been renamed as suggested

In one place I've changed your suggestion:

> For now, I think you should simply place the file directly at:
>
> lisp/progmodes/cperl-mode-tests.el

I guess this was meant to read test/lisp/progmodes/cperl-mode-tests.el.

Also, I've added a ChangeLog-style attachment, as suggested by Lars
Ingebrigtsen in his comment to Bug#42355.
--
Cheers,
haj
[postincrement_1.diff (text/x-patch, attachment)]
[ChangeLog.Bug42168 (text/plain, attachment)]

Added tag(s) patch. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Thu, 13 Aug 2020 20:55:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42168; Package emacs. (Fri, 14 Aug 2020 09:07:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>, 42168 <at> debbugs.gnu.org
Subject: Re: bug#42168: 26.1; cperl-mode: Bad interpretation of $a++ / $b
Date: Fri, 14 Aug 2020 02:06:46 -0700
[Message part 1 (text/plain, inline)]
close 42168 28.1
thanks

Hi Harald,

Harald Jörg <haj <at> posteo.de> writes:

> Thank you for your thorough review.  I'm rather new to Emacs Lisp, and
> also to the Emacs development workflows, so this is highly
> appreciated.

Happy to help, we've all been there at one point.

>> lisp/progmodes/cperl-mode-tests.el
>
> I guess this was meant to read test/lisp/progmodes/cperl-mode-tests.el.

Indeed.

> Also, I've added a ChangeLog-style attachment, as suggested by Lars
> Ingebrigtsen in his comment to Bug#42355.

Thanks.

Your diff looks good, so I've installed the attached patch with some
minor fixes, and I'm closing the bug with this message.

Next time, could you please add the ChangeLog entry to the git commit
message and send the result of `git format-patch -1` as an attachment?
That saves time when installing the change.  Please see the attached for
an example, and you can read more in CONTRIBUTE in emacs.git.  It is
also useful to consult the git log for more examples.

Welcome again as an Emacs contributor!

Best regards,
Stefan Kangas
[0001-cperl-mode-Highlight-a-b-correctly.patch (text/x-diff, attachment)]

bug marked as fixed in version 28.1, send any further explanations to 42168 <at> debbugs.gnu.org and Harald Jörg <haj <at> posteo.de> Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Fri, 14 Aug 2020 09:07:02 GMT) Full text and rfc822 format available.

Severity set to 'minor' from 'normal' Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Wed, 26 Aug 2020 20:11:02 GMT) Full text and rfc822 format available.

Forcibly Merged 36946 42168. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Wed, 26 Aug 2020 20:33:01 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 24 Sep 2020 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 186 days ago.

Previous Next


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