Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short constant names highlighted incorrectly #135

Open
ghost opened this issue May 30, 2018 · 4 comments
Open

Short constant names highlighted incorrectly #135

ghost opened this issue May 30, 2018 · 4 comments
Assignees

Comments

@ghost
Copy link

ghost commented May 30, 2018

Hi. 👋

Short constant names seem to be highlighted incorrectly. I got the assumption that the bug might lie in this MagicPython package. What do you guys think? I ran across this while coding a project, and thought that I should do something about it.

  • Editor name and version: Visual Studio Code 1.23.1
  • Platform: macOS 10.12.6
  • Color scheme: Dracula
  • MagicPython version: Default in Visual Studio Code 1.23.1

VSCode Example

example

GitHub Example

LONGER = "Yeshh!"
B = "What?"

class SomeClass:
    MEMBER = 1
    MEMBER2 = 2
    A = 3
    ARE = 123
    A85 = "Something"

I would also like to give special mention to short constant names that end in numbers. They don't seem to be highlighted correctly, even though the same length character-based constants do.

@ghost ghost changed the title Short Constant Names Highlighted Incorrectly Short constant names highlighted incorrectly May 30, 2018
@vpetrovykh
Copy link
Member

vpetrovykh commented Jul 20, 2018

Sorry for the lengthy silence here.

Oh, the E85 is a good catch and makes sense. I'll fix that one for sure. As to the A, please read the reasons in #42 and #34 for why single upper-case letters don't get highlighted as "constants".

@vpetrovykh vpetrovykh self-assigned this Jul 20, 2018
@vpetrovykh
Copy link
Member

So the issue is that something like E85 is likely to be a constant, but it gets a bit fuzzier with X1. Personally I'm inclined to treat X1 as a constant, but X_1 as a plain variable. I'm somewhat less concerned for heuristincs of something like _1X and it might as well be a "constant" if X1 becomes one.

Please @elprans and @1st1 weight in. Also, hearing from @pikeas (originator for #42) could be useful.

@ghost
Copy link
Author

ghost commented Jul 22, 2018

Thanks for the reply.

This is a tough one, for sure. I can understand the case of A not having enough characters in the name to clearly consider it a constant. It makes sense. I brought it in to the example because I wasn't sure what to think of it.

Below is another real world example of where an inconsistency might come up with longer names. Say we want to declare the instruction set for a CPU (CHIP-8 in this case).

from enum import Enum, auto

class InstructionName(Enum):
	I_0000 = auto()
	I_00E0 = auto()
	I_00EE = auto()
	I_0NNN = auto()
	I_1NNN = auto()
	I_2NNN = auto()
	I_3XKK = auto()
	I_4XKK = auto()
	I_5XY0 = auto()
	I_6XKK = auto()
	I_7XKK = auto()
	I_8XY0 = auto()
	I_8XY1 = auto()
	I_8XY2 = auto()
	I_8XY3 = auto()
	I_8XY4 = auto()
	I_8XY5 = auto()
	I_8XY6 = auto()
	I_8XY7 = auto()
	I_8XYE = auto()
	I_9XY0 = auto()
	I_ANNN = auto()
	I_BNNN = auto()
	I_CXKK = auto()
	I_DXYN = auto()
	I_EX9E = auto()
	I_EXA1 = auto()
	I_FX07 = auto()
	I_FX0A = auto()
	I_FX15 = auto()
	I_FX18 = auto()
	I_FX1E = auto()
	I_FX29 = auto()
	I_FX33 = auto()
	I_FX55 = auto()
	I_FX65 = auto()

There would be no highlighting for I_0000 as it is not considered a proper constant name, even though in this case it clearly can be thought of as one. Also, in regards to the comment from @vpetrovykh, what do you think about this example case, since you mentioned you'd consider X_1 as a plain variable? If X_1 is a plain variable, would X_1234 be one, or should that be considered a constant? Or did I undestand correctly in assuming that you only meant that ruling for two character length names?

I guess the real problem in this ticket is to figure out what kind of contribution digits have to the constant highlighting heuristic when the two character quota for uppercased alphabets can not be met. Whether they should be considered, and if so, what kind of thresholds and constraints should be enforced with them.

@pikeas
Copy link

pikeas commented Jul 24, 2018

Chiming in since I was tagged.

For names containing letters, I suggest the following rule (in pseudocode, not Python):

If the name contains any letters:
    If all letters are uppercase: constant
    Else (at least one letter is lowercase): variable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants