atom / language-python

Python package for Atom
Other
191 stars 147 forks source link

Over zealous and inconsistent highlighting #245

Closed cbarrick closed 4 years ago

cbarrick commented 6 years ago

In short, I oppose some of the changes made in #233. I would like to discuss the pros and cons and hopefully revert to a more conservative tokenization.

The benefits of #233 are simply listed as "✨ highlighting ✨" while the drawbacks are listed as "None". However, I believe that highlighting for the sake of highlighting is not necessarily universally beneficial and can in fact introduce cognitive dissonance. Simply bringing the highlighter "in line with other languages" is not enough; we need to think about Python specifically.

First, attributes are highlighted while locals are not. The issue is, in terms of how they are used, locals and attributes are not different. They can both be accessed, mutated, and overridden using the same syntax. The differences between the two are internal to the interpreter, not to the user.

Second, and related to the first, objects are highlighted differently depending if you are accessing an attribute or not.

Third, attributes are highlighted differently if they are called as methods. This means the same object can be highlighted differently in different cases. It should not be the case that the same type of entity has multiple highlighting.

All of these are particularly annoying when writing in a meta-programming style, which is a major use-case of Python. For example, when looking up a member by string name with getattr(package, 'member'), the package is highlighted differently than with package.member. Or if passing a function around then the fn in foo.fn() is highlighted differently than bar(foo.fn).

I appreciate the effort to add more color, but I don't want color at the cost of consistency. I much prefer the more conservative highlighting of 0.48, and for now I have manually downgraded. But I don't oppose #233 in it's entirety; addressing #141 is great.

Also, I believe this is related to #241.

Examples:

In this code, df is highlighted inconsistently:

python-1

In this code, on line 128, the package is red and the member is orange. But on line 129, the package is orange and the member is red (I believe the highlighter thinks F and SGD are constants when they are not):

python-2
Evpok commented 6 years ago

Note: this does not happen (yet?) using the tree-sitter parser : using it, objects are never highlighted.

cbarrick commented 6 years ago

That's good to hear; it will definitely help.

Perhaps I've raised to broad of an issue for a ticket tracker. Is this the most appropriate forum for this discussion?

winstliu commented 6 years ago

Sorry for the late response. I agree with your points that the fact that member is highlighted differently than member.something is inconsistent. However, this brings language-python in line with the other language packages, such as java, javascript, coffeescript, and php. You could, of course, argue that consistent inconsistency is worse than having consistency in one language that's inconsistent with the others.

Luckily, if you disagree with this change, you can always go back to what it was before by targeting .syntax--variable.syntax--other.syntax--object in your styles.less file.

I'll take a look at the constants issue and possibly revert that change.

cbarrick commented 4 years ago

Closing. I don't see this bug anymore. Not sure when it was fixed.