MagicPython icon indicating copy to clipboard operation
MagicPython copied to clipboard

Short constant names highlighted incorrectly

Open ghost opened this issue 7 years ago • 4 comments

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 avatar May 30 '18 15:05 ghost

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 avatar Jul 20 '18 00:07 vpetrovykh

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.

vpetrovykh avatar Jul 20 '18 00:07 vpetrovykh

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.

ghost avatar Jul 22 '18 10:07 ghost

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

pikeas avatar Jul 24 '18 00:07 pikeas