-
Notifications
You must be signed in to change notification settings - Fork 588
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
[JavaScript] [RFC] Removing readwrite
from variable scopes.
#1336
Comments
From my understanding, the A similar thing was discussed while looking at scope names for annotations: #709 (comment). |
Updating on that, the |
Is there any need for a The downside would be the added difficulty of coloring those variables. But is that useful in the first place? A variable scoped I must admit that I'm skeptical of most of the built-in uses of |
Well, personally I prefer having variables in general not use a special color, since it would become too colorful, but I do like having colors for So, in order to target these "generic" or undecided variables in a scope selector, it'd have to be more specific than just For reference, this is my current color scheme: variable.language,
variable.other \- variable.other.readwrite \- variable.other.object.js \- variable.other.member,
variable.parameter {
@name "Variable";
foreground: #3399CC;
}
// variable \- variable.other.readwrite,
support.variable {
@name "Variable: Support";
foreground: #1786BD;
}
variable.function,
variable.annotation {
@name "Functions (usage) and annotations";
fontStyle: italic;
} |
On a tangential note, I've never really been satisfied with the way that the declaration and use of a name are (not) distinguished. Consider JavaScript (surprise). There are several ways of declaring a name:
We scope these totally differently. In a function or class declaration or expression, we use Once a name is declared, there's no way of knowing where it came from at the site of use. The name The actual use cases we can practically distinguish are:
In particular, we can't say that a given name usage refers to a function parameter or anything like that. And so it seems to me that we have a set of distinctions pertaining to the declaration of a name, and another set of distinctions pertaining to the use of a name, but the former are rudely intruding upon the namespace of the latter. For example, take This might seem like an irrelevant aesthetic quibble, but suppose that the first I wonder if anyone would complain if I went ahead and implemented a set of declaration scopes in JavaScript. Of course, I'd have to maintain backward compatibility — a function parameter might be This really does feel like something that should be standardized. I vaguely remember seeing a suggestion for a standard |
I'm with you on What about dynamically typed languages that don't require a variable to be declared with certain syntax? i.e. |
I don't think there's anything we can do about those languages. The “declaration” is just the first usage, and the first usage is syntactically indistinguishable from any other usage. Syntactically, there's just no declaration to mark. As far as scopes go, something like this for the names:
For entire declarations, possible including multiple names, keywords, and other stuff:
* In JavaScript, function declarations and function expressions look very similar but have subtly different syntax and semantics. Function declarations would be scoped Overall, the intended meaning of |
+1 for uppercase convention.
Did you need to write a python plugin for that? Is the source available? |
@Thom1729 |
Why does it break which color schemes in what ways? |
Lots of color schemes define the color for |
They should define a color for |
I wish this was an isolated case, but lots of color schemes don't and if you change it, it'll have the same fate as changing the object keys to unquoted strings. I am in favor of having precise scopes(like in the object keys example) but at the same time I don't want to make good color schemes look bad. |
I was specifically saying Imho any scheme that does not define a color for |
The issue is that most color schemes assume |
Yeah, the scope naming guidelines specifically call this out:
I'm all for supporting existing color schemes, but we can't refrain from using the standard scopes where they are appropriate just because some color schemes might ignore them. Not when they're on the list of minimal scope coverage for color schemes and when there's an entire section reminding color scheme authors to color that scope in particular. |
This made almost all color schemes assume that |
Not all changes will be backwards compatible. I agree that this situation is unfortunate, but all the color schemes that were only highlighting functions and types were based on https://manual.macromates.com/en/language_grammars#naming_conventions. If we want to make a change and improve scope naming guidelines, we will inevitably have to make changes that cause color schemes to break in minor or major ways. We always try to stay on the minor side, but I really do not see a way to avoid introducing more We have steadily been introducing new entity.name scopes over the past years and nobody has complained ever. Most people are using the default color schemes anyway, which are controlled by Sublime Hq and thus easily changed, so that will cover most users already. The remainder seems to either not care or use a color scheme that matches Regardless, we can add a compatibility layer and assign the old and the new scope to the same token, i.e. |
Because the change probably didn't affect the colors at all; I modified a local syntax to have a This happens because they usually have a rule for
Highly disagree, what I see is the opposite, people install sublime then the first thing they do is to customize the appearance with a theme which in most cases comes with a color scheme that suits the theme. This is in fact why people love sublime text, it's fast and you can make it look the way you want; Just look at the object literal key thread #141 (comment) this was done before and it had to be reverted. Users will wake up someday to find that their code is looking "broken" and then come here asking why. |
I think we should leave variables as plain text, and scope only their declarations. Or at the very least, scope declarations differently from other occurrences. Honestly, it doesn't seem useful to highlight regular occurrences of variables, as it's just about the only part of source code that is not already something fancy, like a keyword, a type name, etc. In fact, I had to disable |
I would agree with scoping variable declarations as |
@keith-hall any specific reason for not considering |
mainly for color scheme reasons, I imagine people would expect variables to all be colored consistently. For example, function definitions that take parameters are "declaring variables", and they get |
I think that using One compromise might be scoping declared variable names as |
Originally, JavaScript did not support constants; all variables could be reassigned. Now, with the adoption of
const
, variables are not necessarily writable. Unfortunately, it is impossible to make this distinction at the time of use. In the statementconsole.log(myVar);
, it is not possible for the syntax to determine whethermyVar
is a read/write variable or a read-only variable. In other words, the JavaScript syntax does not distinguish read/write variables from read-only variables.Currently, all variables are scoped as
variable.other.readwrite
(except in some special cases). This used to be trivially correct, because all JavaScript variables were read/write. Now, it is often incorrect. There is absolutely no way to correctly mark a variable as read/write or readonly, because fundamentally this is not a syntactic distinction.In order to fix this, there is no option but to strip the
readwrite
subscope from the common case and mark variables simply asvariable.other
. We could follow the example of some languages and mark variables beginning with uppercase letters as constants because of common convention, although the language does not enforce this and there may often be exceptions. We can't really mark all-lowercase variables as readwrite because it is very common to have lowercase constants.It would be possible to mark variables as readwrite/readonly when they are declared. However, because this is really a syntactic feature of the entire declaration than of the name itself, I think it would be more appropriate to mark the entire declaration with
meta.declaration.variable.readwrite
or something similar. (This would also mean substantially less code.)Thoughts, objections, alternative suggestions?
The text was updated successfully, but these errors were encountered: