GNU bug report logs - #67390
28; shorthands-font-lock-shorthands assumes shorthand uses same separator

Previous Next

Full log

View this message in rfc822 format

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67390 <at> debbugs.gnu.org, adam <at> alphapapa.net, eliz <at> gnu.org, jonas <at> bernoul.li
Subject: bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator
Date: Sat, 03 Feb 2024 11:43:03 -0800

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

João Távora <joaotavora <at> gmail.com> writes:

> Sorry, this flew under the radar.  I thought I had already pushed to master
> but didn't.  So I went through the commits again, addressed your concerns, and
> applied your suggestions.  Pushed to master now.

Thank you!

> On Sat, Feb 3, 2024 at 7:10 AM Joseph Turner <joseph <at> ushin.org> wrote:
>
>
>>> -[ \t]*%s\\([ \t;]+\\|$\\)")
>>> -                           (regexp-opt (mapcar 'cadr fnlist) t)))
>>> +[ \t]*\\(\\(?:\\sw\\|\\s_\\)+\\)\\([ \t;]+\\|$\\)"))
>>
>> Would you explain what this regexp is intended to match?
>
> A very complicated one, right?  Well ask the author, but I think it's intended
> to find many definition-like forms.  No idea why this is  done with regexps and
> not with 'read' as it is a classical parsing pitfall in the long run.
> Maybe there
> was a reason.
>
> Anyway, I just added a bit of logic so that it considers
> read-symbol-shorthands if
> there are any.

That makes sense.

>> Are you able to reproduce this on your machine?
>
> Yes, and I fixed it.
>
>> ...then this form will allocate two strings almost as long as the file.
>>
>> Here's an alternative hack attempting to uncomment and read the minimum:
>
> Thanks, I think that's a good idea and I added a commit in your name.

Thanks!

>> A couple typo nits on the commit message of "Improve
>> shorthands-font-lock-shorthands (bug#67390)":
>>
>> -  h//thingy     ; hilits "//"       reads to 'hyperdrive--thingy'
>> +  h//thingy     ; hilits "h//"       reads to 'hyperdrive--thingy'
>>
>> -  Co-authored-by: João Távora <joaotavora <at> gmail.com>
>> +  Co-authored-by: Joseph Turner <joseph <at> breatheoutbreathe.in>
>
> I fixed these, too.  If you succesfully test this, I think we can close this bug
> (and the other ones, too).

I'm still reproducing the check-declare bug on my machine. It appears
that binding `enable-local-variables' to nil around the call to
`hack-local-variables' means that `read-symbol-shorthands' is not set.
Can we bind `enable-local-variables' to `:safe' instead?

(let (
      ;; (enable-local-variables t)   ; works
      ;; (enable-local-variables)     ; doesn't work
      (enable-local-variables :safe)  ; works
      )
  (with-temp-buffer
    (insert-file-contents "~/.local/src/hyperdrive.el/hyperdrive-lib.el")
    (hack-local-variables)
    read-symbol-shorthands))

See attached patch.

There's no way to hack just a single file- or dir-local variable, right?

Thank you!

Joseph

[0001-Bind-enable-local-variables-to-safe-for-shorthands-b.patch (text/x-diff, attachment)]

Previous Next

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