Re: master 8f28a1b: Tweak `condition-case' keyword highlights

Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Stefan Monnier]

> +(ert-deftest test-elisp-font-keywords-5 ()
> +  (should (eq (test--font '(condition-case (when a)
> +                               (foo)
> +                             (error t))
> +                          "(\\(when\\)")
> +              nil)))

FWIW, I'd be just as happy if `when` were highlighted with a keyword
face in this case: it's invalid code anyway, so the only highlighting
that would actually be helpful is one with some kind of error/warning
face, but I think it's best to leave it to `flymake-mode` to do that for us.


        Stefan

Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +(ert-deftest test-elisp-font-keywords-5 ()
>> +  (should (eq (test--font '(condition-case (when a)
>> +                               (foo)
>> +                             (error t))
>> +                          "(\\(when\\)")
>> +              nil)))
>
> FWIW, I'd be just as happy if `when` were highlighted with a keyword
> face in this case: it's invalid code anyway, so the only highlighting
> that would actually be helpful is one with some kind of error/warning
> face, but I think it's best to leave it to `flymake-mode` to do that for us.

Sure, I just wanted to tighten the check in that function to the actual
form it was checking, instead of including the VAR form, too.  Nobody
would actually write something like that.

But speaking of other code that's mis-highlighted still:

(defun a (when b c)
  ...)

(The `when' isn't a keyword here, but it's font-locked as such.)

Couldn't we just use `edebug-form-spec' to notice that the arglist isn't
a funcall position?  That would get a huge number of macros/special
forms with non-funcall positions right...
`lisp--el-non-funcall-position-p' would have to parse that, though,
which might slow things down?

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

RE: [External] : Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Drew Adams]

> > FWIW, I'd be just as happy if `when` were highlighted with a keyword
> > face in this case: it's invalid code anyway, so the only highlighting
> > that would actually be helpful is one with some kind of error/warning
> > face, but I think it's best to leave it to `flymake-mode` to do that for
> > us.
> 
> Sure, I just wanted to tighten the check in that function to the actual
> form it was checking, instead of including the VAR form, too.  Nobody
> would actually write something like that.
> 
> But speaking of other code that's mis-highlighted still:
> 
> (defun a (when b c)
>   ...)
> 
> (The `when' isn't a keyword here, but it's font-locked as such.)
> 
> Couldn't we just use `edebug-form-spec' to notice that the arglist isn't
> a funcall position?  That would get a huge number of macros/special
> forms with non-funcall positions right...
> `lisp--el-non-funcall-position-p' would have to parse that, though,
> which might slow things down?

Indeed, such names shouldn't be highlighted when they
don't appear to be used for their defined behavior.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43265#42

It hurts, more than helps, to highlight a defined name
everywhere.

Yes, it might be non-trivial/problematic to correctly
detect normal uses, but at least some first-level
approximation might be helpful.  Blanket highlighting
of such names can be "considered harmful".

https://en.wikipedia.org/wiki/Considered_harmful

Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Couldn't we just use `edebug-form-spec' to notice that the arglist isn't
> a funcall position?  That would get a huge number of macros/special
> forms with non-funcall positions right...
> `lisp--el-non-funcall-position-p' would have to parse that, though,
> which might slow things down?

Looking at this a bit more, parsing the edebug spec should be fast
enough, but we'd then need to know what the position of the current sexp
is in its enclosing form?  `parse-partial-sexp' doesn't return this
data...  Skimming scan_sexps_forward, adding a counter doesn't seem
impossible there?  Would that make sense?

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

Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Stefan Monnier]

> But speaking of other code that's mis-highlighted still:
>
> (defun a (when b c)
>   ...)
>
> (The `when' isn't a keyword here, but it's font-locked as such.)

Yes, that one annoys me fairly often (typically with an arg named `function`).

> Couldn't we just use `edebug-form-spec' to notice that the arglist isn't
> a funcall position?

In theory, yes.  That could cover `condition-case` as well ;-)

I think it'd be nice to use this spec for other things than just
debugging [ I remember a discussion a few .. months? .. back where
another possible use came up... oh yes, it was in the discussion about
preserving source-code location, where we could make use of this info
in order to know when the sexp is "code" which can then come with extra
annotations vs "data" in which case we need the unannotated sexp.  ]


        Stefan

Re: master 8f28a1b: Tweak `condition-case' keyword highlights

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> (The `when' isn't a keyword here, but it's font-locked as such.)
>
> Yes, that one annoys me fairly often (typically with an arg named `function`).

Yeah, me too.

>> Couldn't we just use `edebug-form-spec' to notice that the arglist isn't
>> a funcall position?
>
> In theory, yes.  That could cover `condition-case` as well ;-)

Indeed.  I'm not sure how high our ambitions here are, though -- getting
this right for all forms may be too slow.  I was thinking we'd get 95%
of the way here by just looking at the "simple" form specs.  Let's
see...  what's the spec for `condition-case'...

(symbolp form &rest
         ([&or symbolp
               (&rest symbolp)]
          body))

Well, OK, perhaps that's not unrealistic to apply in an efficient manner.

> I think it'd be nice to use this spec for other things than just
> debugging [ I remember a discussion a few .. months? .. back where
> another possible use came up... oh yes, it was in the discussion about
> preserving source-code location, where we could make use of this info
> in order to know when the sexp is "code" which can then come with extra
> annotations vs "data" in which case we need the unannotated sexp.  ]

Yup.

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