gh-138318, PyREPL: builtins should not be highlighted when used as attribute names

[yihong0618]

We remove REPL highlighting when a builtin name is used as an attribute name. Keywords will still be highlighted as they can never be used as valid attribute names.

---

Previous discussion about keywords

as diff

before the patch:

after the patch:

after the disscuss

• Issue: Built-in names should not be highlighted when used as attribute names #138318

[sobolevn]

I would actually say that the current behavior is more useful, since it allows users to see that they are typing in a keyword name and that there will be a syntax error because of that.

I am -1 on this change, sorry :(

[yihong0618]

I would actually say that the current behavior is more useful, since it allows users to see that they are typing in a keyword name and that there will be a syntax error because of that.

I am -1 on this change, sorry :(

not total agree before this patch I tried
with ipython and other repl for python or other language none of them .xxx highlight

[sobolevn]

I am not against to stop highlighting set in a.set though :)

[picnixz]

with ipython and other repl for python or other language none of them .xxx highlight

That's their own choices. But writing the keyword and hitting enter would anyway raise a SyntaxError so I think it's better that users know that they are writing a keyword and that they can't do that.

---

But I agree that the set in a.set should not be highlighted because it could be very confusing (sometimes, it's necessary to shadow builtins such as format or set)

[yihong0618]

ipython

ghci

with ipython and other repl for python or other language none of them .xxx highlight

That's their own choices. But writing the keyword and hitting enter would anyway raise a SyntaxError so I think it's better that users know that they are writing a keyword and that they can't do that.

But I agree that the set in a.set should not be highlighted because it could be very confusing (sometimes, it's necessary to shadow builtins such as format or set)

a.set and a.list the list highligt in VS Code is strange, I wonder it is the same in repl

[picnixz]

It doesn't matter what IPython does IMO. I would prefer no highlights for built-ins when writing attributes because they can be legitimate ones, as opposed to keywords that can't be used at all. Especially with this kind of constructions:

[yihong0618]

It doesn't matter what IPython does IMO. I would prefer no highlights for built-ins when writing attributes because they can be legitimate ones, as opposed to keywords that can't be used at all. Especially with this kind of constructions:

got you point, I agreed we you do not care what IPython does, I just checked them before this patch to make sure
if I am doing something wrong

[picnixz]

Nonetheless, while two core devs opposed changing the behavior for keywords, I'm willing to support a change of behavior when it comes to built-ins used as attributes. I think it makes sense not to highlight them.

[yihong0618]

Nonetheless, while two core devs opposed changing the behavior for keywords, I'm willing to support a change of behavior when it comes to built-ins used as attributes. I think it makes sense not to highlight them.

thank you

[frostming]

with ipython and other repl for python or other language none of them .xxx highlight

Just chime in to say that IPython does highlight keywords followed by whitespace:

So I agree to only keep the change in builtin names branch.

[yihong0618]

with ipython and other repl for python or other language none of them .xxx highlight

Just chime in to say that IPython does highlight keywords followed by whitespace:

So I agree to only keep the change in builtin names branch.

thank you will only leave buildin

[yihong0618]

now only not highlight the buildin words

[yihong0618]

I am not against to stop highlighting set in a.set though :)

fixed that, thank you.

[sobolevn]

This just has one downside, code like:

>>> some. \
...   set = 1

will still be highlighted. But, I think it is a minor thing?

LGTM :)

[yihong0618]

This just has one downside, code like:

>>> some. \
...   set = 1

will still be highlighted. But, I think it is a minor thing?

LGTM :)

Thank you

[picnixz]

Are we even able to detect those cases in the first place during highlighting?

[yihong0618]

Are we even able to detect those cases in the first place during highlighting?

you mean the case above?

[picnixz]

Yes, I meant the case where we have continuation lines. Do we actually care about this for the other highlights?

[yihong0618]

 some. \
...   set = 1

will try that

[yihong0618]

 some. \

I think we already cover the case can you try?

cc @sobolevn

main

[sobolevn]

@picnixz the common pattern here is to follow back until we find meaningful tokens. Ignoring things like NEWLINE, INDENT and stuff :)

[yihong0618]

@picnixz the common pattern here is to follow back until we find meaningful tokens. Ignoring things like NEWLINE, INDENT and stuff :)

thank you for confirm