GNU bug report logs - #22867
cperl mode highlights %d, but misses %.6f

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 22867 in the body.
You can then email your comments to 22867 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: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: cperl mode highlights %d, but misses %.6f
Date: Tue, 01 Mar 2016 14:52:08 +0800

cperl mode highlights %d, but misses %.6f in
printf "%.6f,%.6f,%d\n" ...

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

From: Alexis <flexibeast <at> gmail.com>
To: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Cc: 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl mode highlights %d, but misses %.6f
Date: Mon, 11 Apr 2016 09:17:08 +1000

積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:

> cperl mode highlights %d, but misses %.6f in printf 
> "%.6f,%.6f,%d\n" ...

Reproduced on emacs-25 branch as at commit 0e7bcec1 
(emacs-version: 25.0.92.2).

Reproduced on master branch as at commit d6ea6453 (emacs-version: 
25.1.50.3).

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl mode highlights %d, but misses %.6f
Date: Fri, 07 Aug 2020 12:09:56 +0200

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

積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:

> cperl mode highlights %d, but misses %.6f in
> printf "%.6f,%.6f,%d\n" ...

[Message part 2 (image/png, inline)]

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

That font locking is actually a bug:

	     ("\\(\\([@%]\\|\\$#\\)[a-zA-Z_:][a-zA-Z0-9_:]*\\)" 1
	      (if (eq (char-after (match-beginning 2)) ?%)
		  'cperl-hash-face
		'cperl-array-face)
	      t)			; arrays and hashes

It's interpreting this as a hash called %d...  but we're inside a
string, so it's not a hash.

At this point, the string has already been fontised...  So how do we
avoid doing this stuff if we're in a string?  Cc'd Stefan for
expertise.  :-)

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

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

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 22867 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#22867: cperl mode highlights %d, but misses %.6f
Date: Fri, 07 Aug 2020 13:13:16 +0200

On Aug 07 2020, Lars Ingebrigtsen wrote:

> That font locking is actually a bug:
>
> 	     ("\\(\\([@%]\\|\\$#\\)[a-zA-Z_:][a-zA-Z0-9_:]*\\)" 1
> 	      (if (eq (char-after (match-beginning 2)) ?%)
> 		  'cperl-hash-face
> 		'cperl-array-face)
> 	      t)			; arrays and hashes
>
> It's interpreting this as a hash called %d...  but we're inside a
> string, so it's not a hash.
>
> At this point, the string has already been fontised...  So how do we
> avoid doing this stuff if we're in a string?  Cc'd Stefan for
> expertise.  :-)

Replace t by nil or 'keep.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 22867 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#22867: cperl mode highlights %d, but misses %.6f
Date: Fri, 07 Aug 2020 14:07:56 +0200

Andreas Schwab <schwab <at> linux-m68k.org> writes:

>> At this point, the string has already been fontised...  So how do we
>> avoid doing this stuff if we're in a string?  Cc'd Stefan for
>> expertise.  :-)
>
> Replace t by nil or 'keep.

Thanks; nil fixed the problem.  (With 'keep %d = {}; wasn't fontified
either.)

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

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

From: Harald Jörg <haj <at> posteo.de>
To: 22867 <at> debbugs.gnu.org
Subject: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array
 declarations
Date: Tue, 1 Sep 2020 18:02:18 +0200

The recent fix d0ad6306 to cperl-mode fixes a lot of unwanted
fontification of arrays and hashes, in particular in comments, POD,
and strings.  However, as a side effect it also prevents fontification
in a variable declaration (this has been observed by choroba):

   my @arr = (1,2,3); # @arr fontified as a scalar - bad
   push @arr, 4;      # @arr fontified as an array - good
   print $arr[4];     # $arr fontified as an array - good

   my %hash = ( foo => 'bar' ); # %hash fontified as a scalar - bad
   $hash{baz => 'quux'};        # $hash fontified as a hash - good

Can this be easily fixed?

---

Of minor importance: There are some cases where fontification of an
array in a string makes sense, but is gone now:

   my $mail = "me <at> somewhere.net";

This was previously fontified as an array.  This makes sense because
in doubly-quoted strings, the value of arrays (here: @somewhere) will
be interpolated into the string.  In the example above, the
fontification of @somewhere as an array was a welcome warning that
this is not what you want.

Fontification of a hash in a string, on the other hand, never makes
sense because hashes aren't interpolated.

---

Also, this bug can be merged with (Bug#11054), which in turn had been
merged with (Bug#3091).

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification
 of hash/array declarations
Date: Tue, 1 Sep 2020 09:34:46 -0700

forcemerge 11054 22867
thanks

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

> The recent fix d0ad6306 to cperl-mode fixes a lot of unwanted
> fontification of arrays and hashes, in particular in comments, POD,
> and strings.  However, as a side effect it also prevents fontification
> in a variable declaration (this has been observed by choroba):

It would be very nice to have test cases for all this.

Is the fix worse than the problem?  If yes, maybe it should be reverted?

> Also, this bug can be merged with (Bug#11054), which in turn had been
> merged with (Bug#3091).

Done.

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

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification
 of hash/array declarations
Date: Tue, 1 Sep 2020 21:44:02 +0200

On 9/1/20 6:34 PM, Stefan Kangas wrote:
> forcemerge 11054 22867
> thanks
> 
> Harald Jörg <haj <at> posteo.de> writes:
> 
>> The recent fix d0ad6306 to cperl-mode fixes a lot of unwanted
>> fontification of arrays and hashes, in particular in comments, POD,
>> and strings.  However, as a side effect it also prevents fontification
>> in a variable declaration (this has been observed by choroba):
> 
> It would be very nice to have test cases for all this.

Indeed.  That will take a while, too :)

Not too long ago I stumbled over (or was pointed to?) a suite
https://github.com/Lindydancer/font-lock-regression-suite which also
contains an example in Perl.  Testing this example against its
faceup-counterpart would fail as of today.

> Is the fix worse than the problem?  If yes, maybe it should be reverted?

The fix isn't _worse_.  Strings like "%s" and "%d" in the argument
list of printf being fontified as hashes is indeed idiotic.  Also,
more than once I was annoyed by fontification in POD sections.

On the other hand, declarations of hashes and arrays occur rather
frequently, and there's a lot more in that area that would deserve
some love and care.  So, while this fix was a necessary first step in
any case, some refinement might be desirable to cut off the rough
edges it left.  I don't see an easy way, but this might be well due to
my limited elisp experience.
-- 
Cheers,
haj

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Harald Jörg <haj <at> posteo.de>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification
 of hash/array declarations
Date: Fri, 04 Sep 2020 05:46:03 +0200

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

>> Is the fix worse than the problem?  If yes, maybe it should be reverted?
>
> The fix isn't _worse_.  Strings like "%s" and "%d" in the argument
> list of printf being fontified as hashes is indeed idiotic.  Also,
> more than once I was annoyed by fontification in POD sections.

The change in the offending commit just ensured that we don't re-fontify
already-fontified stuff as hashes.  This means that the reason stuff
like "my %foo = ..." isn't fontified as a hash is because something has
already fontified it as something else, so I wondered whether just
flipping the order of some of these regexp would do the trick.  After
poking around a bit, I came up with this patch:

diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 5dee5007e2..7a1c2e4d24 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -5776,8 +5776,8 @@ cperl-init-faces
 		  t-font-lock-keywords)
 		cperl-font-lock-keywords cperl-font-lock-keywords-1
 		cperl-font-lock-keywords-2 (append
-					   cperl-font-lock-keywords-1
-					   t-font-lock-keywords-1)))
+					   t-font-lock-keywords-1
+					   cperl-font-lock-keywords-1)))
 	(if (fboundp 'ps-print-buffer) (cperl-ps-print-init))
 	(if (or (featurep 'choose-color) (featurep 'font-lock-extra))
 	    (eval			; Avoid a warning


It seems to fix all the test cases in Harald's .pl file, and the cperl
test file still passes (but its coverage isn't, ahem, extensive).

Does anybody see any problems with just doing fixing it like this?  I
had a peek at a couple of other perl files here, and nothing immediately
looked wonky, but I'm a bit out of practice writing perl...

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

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

From: Harald Jörg <haj <at> posteo.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification
 of hash/array declarations
Date: Fri, 4 Sep 2020 18:04:07 +0200

On 9/4/20 5:46 AM, Lars Ingebrigtsen wrote:
> Harald Jörg <haj <at> posteo.de> writes:
> 
>>> Is the fix worse than the problem?  If yes, maybe it should be reverted?
>>
>> The fix isn't _worse_.  Strings like "%s" and "%d" in the argument
>> list of printf being fontified as hashes is indeed idiotic.  Also,
>> more than once I was annoyed by fontification in POD sections.
> [...]
> After poking around a bit, I came up with this patch:
> 
> diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
> index 5dee5007e2..7a1c2e4d24 100644
> --- a/lisp/progmodes/cperl-mode.el
> +++ b/lisp/progmodes/cperl-mode.el
> @@ -5776,8 +5776,8 @@ cperl-init-faces
>  		  t-font-lock-keywords)
>  		cperl-font-lock-keywords cperl-font-lock-keywords-1
>  		cperl-font-lock-keywords-2 (append
> -					   cperl-font-lock-keywords-1
> -					   t-font-lock-keywords-1)))
> +					   t-font-lock-keywords-1
> +					   cperl-font-lock-keywords-1)))
>  	(if (fboundp 'ps-print-buffer) (cperl-ps-print-init))
>  	(if (or (featurep 'choose-color) (featurep 'font-lock-extra))
>  	    (eval			; Avoid a warning
> 
> 
> It seems to fix all the test cases in Harald's .pl file, and the cperl
> test file still passes (but its coverage isn't, ahem, extensive).
> 
> Does anybody see any problems with just doing fixing it like this?  I
> had a peek at a couple of other perl files here, and nothing immediately
> looked wonky, but I'm a bit out of practice writing perl...

That change is fine with me!
-- 
Cheers,
haj

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Harald Jörg <haj <at> posteo.de>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 22867 <at> debbugs.gnu.org
Subject: Re: bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification
 of hash/array declarations
Date: Sat, 05 Sep 2020 14:37:11 +0200

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

>> Does anybody see any problems with just doing fixing it like this?  I
>> had a peek at a couple of other perl files here, and nothing immediately
>> looked wonky, but I'm a bit out of practice writing perl...
>
> That change is fine with me!

OK; applied to Emacs 28.  

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

Previous Next

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