|
From: | Yuan Fu |
Subject: | bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode |
Date: | Sun, 10 Dec 2023 23:10:44 -0800 |
User-agent: | Mozilla Thunderbird |
On 12/10/23 4:00 PM, Dmitry Gutov wrote:
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.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.
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.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.
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.
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.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.
Yuan
[Prev in Thread] | Current Thread | [Next in Thread] |