* bug#22867: cperl mode highlights %d, but misses %.6f
@ 2016-03-01 6:52 積丹尼 Dan Jacobson
2016-04-10 23:17 ` Alexis
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: 積丹尼 Dan Jacobson @ 2016-03-01 6:52 UTC (permalink / raw)
To: 22867
cperl mode highlights %d, but misses %.6f in
printf "%.6f,%.6f,%d\n" ...
^ permalink raw reply [flat|nested] 11+ messages in thread* bug#22867: cperl mode highlights %d, but misses %.6f
2016-03-01 6:52 bug#22867: cperl mode highlights %d, but misses %.6f 積丹尼 Dan Jacobson
@ 2016-04-10 23:17 ` Alexis
2020-08-07 10:09 ` Lars Ingebrigtsen
2020-09-01 16:02 ` bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations Harald Jörg
2 siblings, 0 replies; 11+ messages in thread
From: Alexis @ 2016-04-10 23:17 UTC (permalink / raw)
To: 積丹尼 Dan Jacobson; +Cc: 22867
積丹尼 Dan Jacobson <jidanni@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).
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#22867: cperl mode highlights %d, but misses %.6f
2016-03-01 6:52 bug#22867: cperl mode highlights %d, but misses %.6f 積丹尼 Dan Jacobson
2016-04-10 23:17 ` Alexis
@ 2020-08-07 10:09 ` Lars Ingebrigtsen
2020-08-07 11:13 ` Andreas Schwab
2020-09-01 16:02 ` bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations Harald Jörg
2 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-07 10:09 UTC (permalink / raw)
To: 積丹尼 Dan Jacobson; +Cc: Stefan Monnier, 22867
[-- Attachment #1: Type: text/plain, Size: 137 bytes --]
積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:
> cperl mode highlights %d, but misses %.6f in
> printf "%.6f,%.6f,%d\n" ...
[-- Attachment #2: Type: image/png, Size: 8309 bytes --]
[-- Attachment #3: Type: text/plain, Size: 571 bytes --]
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
^ permalink raw reply [flat|nested] 11+ messages in thread* bug#22867: cperl mode highlights %d, but misses %.6f
2020-08-07 10:09 ` Lars Ingebrigtsen
@ 2020-08-07 11:13 ` Andreas Schwab
2020-08-07 12:07 ` Lars Ingebrigtsen
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2020-08-07 11:13 UTC (permalink / raw)
To: Lars Ingebrigtsen
Cc: Stefan Monnier, 22867, 積丹尼 Dan Jacobson
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@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread* bug#22867: cperl mode highlights %d, but misses %.6f
2020-08-07 11:13 ` Andreas Schwab
@ 2020-08-07 12:07 ` Lars Ingebrigtsen
0 siblings, 0 replies; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-07 12:07 UTC (permalink / raw)
To: Andreas Schwab
Cc: Stefan Monnier, 22867, 積丹尼 Dan Jacobson
Andreas Schwab <schwab@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2016-03-01 6:52 bug#22867: cperl mode highlights %d, but misses %.6f 積丹尼 Dan Jacobson
2016-04-10 23:17 ` Alexis
2020-08-07 10:09 ` Lars Ingebrigtsen
@ 2020-09-01 16:02 ` Harald Jörg
2020-09-01 16:34 ` Stefan Kangas
2 siblings, 1 reply; 11+ messages in thread
From: Harald Jörg @ 2020-09-01 16:02 UTC (permalink / raw)
To: 22867
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@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).
^ permalink raw reply [flat|nested] 11+ messages in thread* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2020-09-01 16:02 ` bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations Harald Jörg
@ 2020-09-01 16:34 ` Stefan Kangas
2020-09-01 19:44 ` Harald Jörg
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2020-09-01 16:34 UTC (permalink / raw)
To: Harald Jörg, 22867
forcemerge 11054 22867
thanks
Harald Jörg <haj@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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2020-09-01 16:34 ` Stefan Kangas
@ 2020-09-01 19:44 ` Harald Jörg
2020-09-04 3:46 ` Lars Ingebrigtsen
0 siblings, 1 reply; 11+ messages in thread
From: Harald Jörg @ 2020-09-01 19:44 UTC (permalink / raw)
To: Stefan Kangas, 22867
On 9/1/20 6:34 PM, Stefan Kangas wrote:
> forcemerge 11054 22867
> thanks
>
> Harald Jörg <haj@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2020-09-01 19:44 ` Harald Jörg
@ 2020-09-04 3:46 ` Lars Ingebrigtsen
2020-09-04 16:04 ` Harald Jörg
0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-04 3:46 UTC (permalink / raw)
To: Harald Jörg; +Cc: Stefan Kangas, 22867
Harald Jörg <haj@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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2020-09-04 3:46 ` Lars Ingebrigtsen
@ 2020-09-04 16:04 ` Harald Jörg
2020-09-05 12:37 ` Lars Ingebrigtsen
0 siblings, 1 reply; 11+ messages in thread
From: Harald Jörg @ 2020-09-04 16:04 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: Stefan Kangas, 22867
On 9/4/20 5:46 AM, Lars Ingebrigtsen wrote:
> Harald Jörg <haj@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations
2020-09-04 16:04 ` Harald Jörg
@ 2020-09-05 12:37 ` Lars Ingebrigtsen
0 siblings, 0 replies; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-05 12:37 UTC (permalink / raw)
To: Harald Jörg; +Cc: Stefan Kangas, 22867
Harald Jörg <haj@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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-09-05 12:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-01 6:52 bug#22867: cperl mode highlights %d, but misses %.6f 積丹尼 Dan Jacobson
2016-04-10 23:17 ` Alexis
2020-08-07 10:09 ` Lars Ingebrigtsen
2020-08-07 11:13 ` Andreas Schwab
2020-08-07 12:07 ` Lars Ingebrigtsen
2020-09-01 16:02 ` bug#22867: cperl-mode: Commit d0ad6306 suppresses fontification of hash/array declarations Harald Jörg
2020-09-01 16:34 ` Stefan Kangas
2020-09-01 19:44 ` Harald Jörg
2020-09-04 3:46 ` Lars Ingebrigtsen
2020-09-04 16:04 ` Harald Jörg
2020-09-05 12:37 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).