all color themes: treat comment docstrings as comments too #182162
Conversation
|
|
It makes sense to render docstrings as comments. Would you mind adding that rule to the other built-in themes as well? |
ElectricRCAircraftGuy
force-pushed
the
gstaples_fix_python_comment_strings
branch
from
776545b
to
44c2f52
Compare
|
@microsoft-github-policy-service agree |
@aeschli , done. I'm grateful you received this PR positively. |
|
Thanks! |
...`"string.quoted.docstring"`, instead of the more-narrow, Python-only `"string.quoted.docstring.multi.python"`, per the recommendation in this comment here: microsoft/vscode#182162 (comment)
ElectricRCAircraftGuy
changed the title
...instead of "string.quoted.docstring.multi.python", per @aeschli's request here: microsoft#182162 (comment) This way the comment formatting applies to *all* language quoted docstrings, instead of just to Python.
...as comments too
modified: extensions/theme-abyss/themes/abyss-color-theme.json modified: extensions/theme-defaults/themes/dark_vs.json modified: extensions/theme-defaults/themes/hc_black.json modified: extensions/theme-defaults/themes/hc_light.json modified: extensions/theme-defaults/themes/light_vs.json modified: extensions/theme-kimbie-dark/themes/kimbie-dark-color-theme.json modified: extensions/theme-monokai-dimmed/themes/dimmed-monokai-color-theme.json modified: extensions/theme-monokai/themes/monokai-color-theme.json modified: extensions/theme-quietlight/themes/quietlight-color-theme.json modified: extensions/theme-red/themes/Red-color-theme.json modified: extensions/theme-solarized-dark/themes/solarized-dark-color-theme.json modified: extensions/theme-solarized-light/themes/solarized-light-color-theme.json modified: extensions/theme-tomorrow-night-blue/themes/tomorrow-night-blue-color-theme.json
...instead of "string.quoted.docstring.multi.python", per @aeschli's request here: microsoft#182162 (comment) This way the comment formatting applies to *all* language quoted docstrings, instead of just to Python.
ElectricRCAircraftGuy
force-pushed
the
gstaples_fix_python_comment_strings
branch
from
1ab46a0
to
f43e8a6
Compare
@aeschli , done. I hope you'll approve it now. I also tested it on the Python code and it did still work right. |
|
How do I reverse this locally? I preferred it the way it was. Sudden and unexplained changes after a restart are far more obnoxious than the orange color -- at least it looked good, which I can't say for this ugly dark green. Edit 2: finally figured out a solution that will not simply revert when the dark_vs.json file resets. Edit the "editor.tokenColorCustomizations": {
"[*]": {
"textMateRules": [
{
"name": "Docstrings",
"scope": "string.quoted.docstring",
"settings": {
"foreground": "#CE9178"
}
}
]
}
}Had to use an asterisk because the theme names are aliased in some obscure fashion, and those aliases are not accessible via suggestions, so this will affect all themes. Putting in the Edit: I figured it out. The color is coming from the dark_vs.json theme file, whether or not you have that theme selected. The problem was that in the "tokenColors" array was the following object (outdated): {
"name": "Comments",
"scope": ["comment", "string.quoted.docstring"],
"settings": { "foreground": "#6a9955" }
},To restore all dark themes back to the way they were a couple hours ago, I replaced it with the following: {
"name": "Docstrings",
"scope": ["string.quoted.docstring"],
"settings": { "foreground": "#ce9178" }
},
{
"name": "Comments",
"scope": ["comment"],
"settings": { "foreground": "#6a9955" }
},This change requires a restart to take effect. Leaving this here for anyone else who got blindsided by this and wanted to revert it. |
|
Related on Stack Overflow:
Help with closing the duplicates would be appreciated so that the answers all get put in one place for future readers. |
|
Very much agreed that sudden and unexplained changes like this, which ultimately boil down to purely someone's personal preference, are quite obnoxious. While the change makes conceptually sense, it would be better in future to maintain current behaviour while enacting the change. |
I'm not with VSCode, but I'm wondering: how does one maintain current behavior while enacting the change? They seem so me to be mutually exclusive events. (Literally, if you have a way how, please explain how. I don't see how this request is possible without adding a menu setting to "enable correct syntax highlighting", which isn't practical). Part of the pain is that the syntax highlighting schemes may need to be updated once the change happens, too, but it's not practical to expect that to happen at the same time, nor does it make sense to update all schemes to undo the very behavior this change makes. I think it's just an unfortunate side effect of having wrong behavior that some people will prefer the wrong behavior to continue because they got used to it. |
|
To help people go back: How to colorize Python docstrings to your liking; ex: like strings <-- click here to see how to go back to the old settingsFor anyone wishing to customize your theme to go back to how it was for your syntax highlighter, read my answer here to learn how to do that. It contains info about finding the names and colors of sections you like and how to change the ones you don't: How to subdue Python block comment strings in VSCode in order to look like Sublime Text's Monokai In my link above, I explain how to use the token and scope inspector, and then I say:
@starball5 also pointed out this Q&A: Restarted Visual Studio Code, and now Python docstrings are an ugly dark green. How do I return it to the original orange?, and has posted a great answer to it here. |
In this particular case it should've been doable to automatically generate a diff between the current user's theme settings and what the result of this change would be and to store the diff as explained in the other posts in the user's settings. I.e. automate pretty much what we now all did manually. I'm pretty sure I've seen updates to VSCode do this before... I'm not saying that's necessarily easy or that I'd even know how to do it, but personally I wouldn't even attempt such a change without the above prophylaxis.
That's the key observation however: people got used to it. Ultimately VSCode is just a tool and comfort in using it supersedes any abstract desire for "conceptually correct behaviour". I'm usually all for the latter, but sudden changes to something that's ultimately anyways customizable and user-specific is just a no-go for me. |
@ElectricRCAircraftGuy @aeschli I use docstrings as docstrings, containing information, e.g., for functions/methods, the summary of functionality and signature. These should be more important than just a "comment" and be highlighted differently. |
However, the problem is no one commented with an objection before it was merged. |
|
FWIW, I saw the title of this PR in the release notes and thought, "Finally!" I am very much +1 on this change. I understand that semantically, docstrings and comments aren't the same thing, but IMHO they are used for enough of the same purpose (documentation ignored by the interpreter*) that they should be lumped together, and should both be dimmed, since they don't affect the behavior of the function/method/module/whatever. Conversely, docstrings, while they are not comments, are also not strings in the normal sense, and therefore should not be colored the same way. *I'm leaving aside the issue of doctests, which of course the interpreter does handle, but AFAIK they're not colored differently than any other part of a docstring, so for the purposes of this conversation I'm lumping them in with non-runnable docstring content. |
The interpreter does not actually ignore them in Python -- it puts them on a special |
Okay, yes, fair enough, but for the general running of whatever code I'm writing, they function much more like comments than like values, i.e., the 99% case is that my code doesn't involve |
Seems we have different opinions on docstrings. I think docstrings exist to help us figure out what it does without reading through code (especially when we just want to use a function/class etc. and don't care about the internals). |
Agreed. docstrings are documentation and not comments. There are python projects such as
If a project is using docstrings as a place for comments, then I'd suggest the developers use comments for comments and docstrings for documentation. |
|
This PR was not the way to go about "fixing" this. If the convention for docstrings was to treat them like comments and not documentation that is actually functionally accessible text, then we would use comment notation. Even PEP8 recommends multiline comments to begin with |
|
I feel like we're missing the forest for the trees here. We all agree that string literals, docstrings, and comments are each their own thing, with different (albeit often overlapping) uses, syntax, integration with tooling, etc. One option is to make all three different tokens (and therefore potentially all three different colors), as proposed in #184836. Another option is to lump docstrings in with either string literals (as has been the case up 'til now) or comments (as is the case after this change). The implicit point of this PR, which I'm also arguing for, is that in the end, docstrings are documentation. They're English sentences. They're explanations. Changing them doesn't change how a function works, or the result it gives. This makes them fundamentally different than literal values (string or otherwise), but very much akin to comments. (This is why in JS they're grey, just like comments, and always have been.) Bottom line, in the same way that I want to be able to tune out comments, I want to be able to tune out docstrings, too. I want them both to be there, each serving their purpose, available if I want them, but neither one grabbing my attention. And the most straightforward way to do that is to dim them both, which is what this PR does. |
I feel like you're just reiterating your personal preference? The pure facts are that two basic mistakes are being or have been committed here:
Each one of us, both before the PR under discussion and after, can set things up the way they like: making docstrings look like comments or not. But PR's like this one should not change the look of a user's IDE on update and ideally should implement "conceptually correct" behaviour once and for all. P.S.: I'm assuming above that docstrings were separately configurable before this PR. If this was not the case, then this PR has some merit on that front. Either way, would be much better if it implemented conceptually sound behaviour to begin with as apposed to an approximation. |
Kinda? This PR was open for four days before it was merged (May 11th until 15th). That didn't leave a lot of time for people to comment on a rather substantial change to the UX. I also can't see any attempt to gather input from the Python community, either in general or even the developers of the vscode-python or related extensions. The issue which the PR was marked as fixing was opened after the PR was, implying that no prior requests for this change existed (a quick skim of the issues on this repo seems to agree). An issue (microsoft/pylance-release#4429) was raised on the 22nd May reporting this as a bug in the insiders build of VSCode, though unfortunately it was combined with a more general syntax highlighting issue and misattributed to pylance rather than VSCode itself. The discussion there suggests that even internal Microsoft folks were unaware of this change!
Looking at the diff of this PR and the workarounds which people are applying to fix this issue, customising this per-user seems to have already been possible. This change only flips the default so that VSCode itself now treats some Python strings as comments. A conflating factor here is that the parser seems to be imprecise in what it considers docstrings, at least relative to what the interpreter will actually attach to a def foo():
"docstring" # ends up in `foo.__doc__`, now considered a comment
print(2)
"not a docstring" # but still now considered a comment
returnPersonally I would ask the VSCode team to:
|
Agreed. That's the point I've been making: we should make things "conceptually correct"; if we can't (for the moment), we should choose the way that minimizes UX impact.
Yes, they are, just like the workaround (see SO links above). However, all "literal string statements" currently share the token for "docstrings", and we can only render them as either docstrings or comments.
I don't know how many people are the same, but for me, I actually only looked into this repo after running into the current problem. However, a longer time may still help more people come across this PR.
This is indeed a problem. But it could be because this is treated as a bugfix instead of a feature change.
Actually, I sometimes do the similar thing if there's a quick bugfix. I think the core problem here is whether this is a bugfix (which only improves things) or a feature change (which may have a negative impact, at least on part of the users). It's better to gather community inputs before a feature change, while bugfixes may be merged through a faster track. Around this issue, the author thinks this is a bugfix, but I prefer to take it as a feature change. However, the maintainers should rectify how to categorize. |
I have summarized the behavior in #184836, Python itself has definitions, and the python language server (pylance) has its own interpretation. Therefore I'm proposing to add new tokens and let the language server decide how they're assigned to every string. For your convenience, I paste the relevant information here:
That is, only the first statement in a scope can be a "docstring". Python extension of vscode: class A:
"""This is the docstring for A. Shown on hover."""
B = 1
"""Shown on hover for B."""
def c(self, x:int)->int:
"""This is the docstring for c. Shown on hover."""
d = 1
"""This is nothing but a comment."""
e = 1
"""Shown on hover for e."""For Slightly off-topic
MS is quite large. I think it's normal that the left hand is unaware of what the right hand is doing. |
Here's what I meant: Trees: Docstrings and comments are not the same thing, for x, y, z technical reasons. (I think we all agree here.) Other than the questions around process (which are valid, but separate), the only part that's opinion/preference is which one of those two is most important. And on that topic I feel like we've all expressed our opinions here both thoroughly and cogently, so at this point I will leave this in the hands of the maintainers. |
I'd swap the forest and trees in your example. Ultimately the editor needs to respect the python language and the facts are that:
The fact that docstrings are documentation is true, however docstrings are customer facing documentation which are python objects whereas code comments are internally facing (and best practice is that comments are not python objects such as string) - hence the importance of being able to distinguish them from each other. I share the opinion of others in this thread that this PR should be reverted. I also think that this PR does not respect what is most significant for the python color themes in VSCode: distinguishing the different features of the python language to the developer. |
Testing the limits of this statement shows why this argument doesn't hold water for formatting things which are of documentation in nature as the same colour... Trivial code: class IceCream:
"""Yummy ice cream!"""
def __init__(self, flavour):
# Ice cream is typically served cold
self.temperature_deg_c = -15
# Support any flavour
self.flavour = flavour
class IceCreamMachine(Machine):
"""An ice cream machine which dispenses delicious ice cream!"""
def dispense(self, n, flavour):
"""Dispenses n number of icecreams in the requested flavour.
Parameters
----------
n: int
Number of ice creams to dispense.
flavour: str
Any flavour you like so long as it is string.
Returns
-------
list of IceCream
IceCream class instances
"""
# Catch any non-str flavour
if flavour is not str:
raise TypeError("Flavour must be str.")
# Create n instances of requested flavour
icecreams = [IceCream(flavour) for i in range(n)]
return icecreamsNow testing the upper limit of how we can "comment" our code: class IceCream:
"""Yummy ice cream!"""
def __init__(self, flavour):
comment = "Ice_cream_is_typically_served_cold"
self.temperature_deg_c = -15
Support_any_flavour = "comment"
self.flavour = flavour
class IceCreamMachine(Machine):
"""An ice cream machine which dispenses delicious ice cream!"""
def dispense(self, n, flavour):
"""Dispenses n number of icecreams in the requested flavour.
Parameters
----------
n: int
Number of ice creams to dispense.
flavour: str
Any flavour you like so long as it is string.
Returns
-------
list of IceCream
IceCream class instances
"""
Catch_any_non_str_flavour = "comment"
if flavour is not str:
raise TypeError("Flavour must be str.")
"Create n instances of requested flavour"
icecreams = [IceCream(flavour) for i in range(n)]
return icecreamsComment: Ok now we are using variables to comment our code, it's not python best practice but it is effectively documenting what the code is doing. Those variable assignments which are documentation should now be color formatted the same as code comments. Using variable assignments this way creates an object as documentation much the same as using unassigned str to document does. Now testing the lower limit of how we can document our code: class IceCream:
def __init__(self, flavour):
self.temperature_deg_c = -15
self.flavour = flavour
class IceCreamMachine(Machine):
def dispense(self, n, flavour):
if flavour is not str:
raise TypeError("Flavour must be str.")
icecreams = [IceCream(flavour) for i in range(n)]
return icecreamsComment: In this case without the use of docstrings, unassigned str, comments or "comment" variable assignments we have sufficiently (though minimally) documented code through careful selection of class, attribute, method names and variable assignments. Python code is often written in ways where it ends up documenting itself. Should this also be formatted the same colour as code comments? |
To summarise this demonstration shows that:
Hence, we should not colour docstring/comment/variables with our inference of intent (such as documentation) but color them by what they are as features of the python language (otherwise everything will be coloured the same). |
|
Dear @ElectricRCAircraftGuy, could you please revert back to the old color scheme where docstrings had a distinct color for easier readability. This is beneficial and user-friendly for programmers' eyes. It's hard to believe that such a terrible request was approved, causing this disruption. |
|
Reminder, here is how to fix the colors for yourself: If you do this change and then click the little person icon in the bottom-left of VSCode, and sign in with GitHub (or one of the other sign-in options) and turn on sync, it will apply this change to all instances of VSCode you have, on all computers you have. |
|
hi @ElectricRCAircraftGuy |
|
dear @ElectricRCAircraftGuy |
@minhaics , what color scheme do you have? I'll give you the code. The color you had is the color of a string. Open a Python file and type |
|
This PR is based on the wrong assumption that in Python, tripple-quote strings that are not assigned to a variable are "block comments". This is not correct. It is just a string that is not assigned. Python doesn't have any true options for multi-line comments, like True, some people highjack tripple-quotes to quickly comment out blocks of code, but that is super flaky. For instance if the code you want to comment out already contains tripple-quoted strings. Or if used inside brackets (where python will concatenate strings if they are only separated by newlines and whitespace). The proper way to comment out parts of code in Python is to highlight the lines you want to comment out and ask your IDE to comment the selected lines (ctrl + / in vscode). The Other questions:
All in all, I understand the rationale for this PR, but it just seems like it can have too many unexpected/unintended side-effects or edge-cases. Which is what can easily happen when you change the interpretation of basic parts of a language. And, as everyone else have already pointed out: Docstrings, in particular, are certainly not comments, since they affect the output being produced by the interpreter. They should probably have their own color-theme option in the editor. And if there is a desire to have other unassigned strings appear different from normal strings, then there should maybe be a separate token for unassigned strings (that are not docstrings). And then let the user decide which color each of those should be. If you want your docstrings to be colored the same color as comments, you can pick a theme that does that. P.S: Regarding the best way to implement changes like these, that mostly comes down to personal preference: There is a reason that this "small change" has received such a major blowback. Whenever you change important parts of the UI, such as syntax highlighting, there is a mental overhead to users to adapt to the new change. So you should be really, really careful about applying changes to the UI, especially when the change is mostly a matter of personal preference. If you want to avoid drama, the best approach is generally to first make it possible to apply the change as a user setting (e.g. by changing the theme). And then, after some time, see if a majority of users prefer the new change, and then maybe consider making that the new default. In this case, we should have first added support for styling docstrings and unassigned strings separately from normal strings. Then created some new themes that would do just that, maybe one that would color docstrings the same color as comments, and another theme that would color docstrings differently from either comments and normal strings. And then see if those new themes suddenly become more popular. And if they do, consider changing the default theme to one of these, but leaving the old theme in place for people who prefer the old theme. |
|
Hi @ElectricRCAircraftGuy |
|
Hi @ElectricRCAircraftGuy |
This treats Python comment block docstrings as comments too, so they are subdued, like in Sublime Text, so we can focus on the code.
Before this change (bad, because the Python block comment strings are an obnoxious yellow):
After (good, because now they are treated like comments, and subdued):
Notice that regular multi-line block strings (
strin the code above) still show up in yellow, as they should.I wrote about this here: How to subdue Python block comment strings in VSCode in order to look like Sublime Text's Monokai
Fixes #182163