Subject: Re: bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

From: Yuan Fu <[email-masked]>
Date: Sun, 10 Dec 2023 23:10:44 -0800

On 12/10/23 4:00 PM, Dmitry Gutov wrote:
> On 10/12/2023 14:04, Denis Zubarev wrote:
>>  > Arguably, the last 2 lines are "variable references" rather than 
>> definitions
>> `var := 3`  is assignment expressions. It allows variable assignments 
>> to occur inside of larger expressions. For example `if (match := 
>> pattern.search(data)) is not None:`.
>> It mostly used to define new variables and act on them if some 
>> condition is met.
>
> Ah, thanks! This feature is newer than my working experience with Python.
>
>> My rationale for `var *= 3` was that it is shorthand for `var = var * 
>> 3` and currently the `var` on the left hand side is fontified with 
>> `font-lock-variable-name-face`.
>
> I think ideally, in cases when "var =" is not the first occurrence for 
> the same var in a given scope, we wouldn't highlight it as 
> "definition" either.
>
> Speaking of font-lock-variable-use-face, I think it would be most 
> useful if it helped with noticing typos (meaning, it would only be 
> used for references to variables that have been defined previously, 
> according to the rules of the language). But for that we still need to 
> implement the scope tracking. And before that, well, my personal 
> preference is not to highlight the references at all, but opinions 
> differ.
Personally I regard the "assignment" feature to mean "any assignment", 
rather than definition. But that's just me. For my personal taste, I 
would make *= always highlight its LHS.
>> I wanted shorthand form to be consistent with the full form.
>> Your point makes sense too, I don't have strong opinion about this.
>> Also I'm not sure now about `var[ii] = 1`, since it is actually 
>> accessing the list or dictionary element and 
>> `font-lock-variable-use-face` may suit better here.
>
> Yep. To sum up, I would add highlighting to your examples `for var in 
> range(3)` and `var := 3` but not others.
IMHO, for the assignment feature, we should stick to the narrow 
definition of assignments, ie, anything that looks like "a = b". Things 
like "for var in range(3)" could be highlighted by variable feature, I 
think.

And for var[i] = 1, I don't know either. I think I decided to not 
fontify it back then, but it wasn't based on any strong reasoning.
>> Question about new changes.
>> Should I push them to this patch and provide description of new changes,
>> or it would be better to wait for review and send them as new patch?
>
> I suggest sending an updated patch for review in this case, but you 
> can also wait for Yuan's comments first.
>
If you can consolidate everything into a single patch, and pair it with 
a summary, that'll be a great aid to me. Also, in case you don't know 
yet, we follow certain format for commit messages, you can check it out 
in the CONTRIBUTE file, "Commit messages" section.

Yuan