Revert Python docstring color #184938
Revert Python docstring color #184938
Conversation
There was a problem hiding this comment.
Viewed each theme with a test file.
Themes appear to follow standard syntactic highlight consistent with PEP8 and PEP257 returning docstrings to current proper convention.
Test Code:
#!/usr/bin/env python3
'''
File header documentation
Testing syntactic highlighting
'''
from __future__ import with_statement as fws
print('Hello World!')
def foo(unused_args=None, used_args=None):
'''
Useless test function. Users need to see this information
This information is functional and important to the programmer
@param unused_args: any, unused argument
@param used_args: any, returned argument(s)
@return: used_args argument is returned. Seems useless
'''
# Why implement this def??? <- nobody needs to see that
# Hey, look at the GitHub syntax highlighting!
used_args = None
return used_args
foo()
help(foo)
class Bar(object):
'''Class docstring'''
def __init__(self) -> None:
pass
thing = Bar()
CONST = 0
var = 1
True or False
'''
Non-standard block comment
'''
# standard
# block
# comment
# according
# to PEP8
There was a problem hiding this comment.
As a revert to the original behaviour, this looks good to me.
As a commentary on the idea that both original and new behaviours are wrong - the recent "make docstrings look like comments" is very wrong.
I'd disagree however in saying docstring should differentiate visually from a string.
I would have also been happy with singling out docstring as an independent configurable type, and defaulting it to match string coloring, independently of comments and regular strings.
|
i got the same issue |
|
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. |
|
@aeschli is there anything that members of the community can do to help progress this PR? It looks like quite a few people have validated that this achieves the expected result (i.e: reverting back to the previous behaviour). I think it's fair to say that where we go from there is more complicated, though reverting to the previous behaviour feels the best option given how contentious this is. That would then give the VSCode team (along with the VSCode-Python team and the community) time to discuss the options and work out a migration strategy for affected users of whatever change happens (if any change happens). |
|
hi @PeterJCLaw |
|
@minhaics "PR" is a common abbreviation for "Pull Request". |
|
What can be done to help this PR get pushed through? |
|
I find it quite shocking that an incorrect and unwanted change was merged in just 4 days and with only 2 reviewers, but an almost universally agreed upon correction is taking more than a month to merge and needs apparently 11 reviewers?!?! The community doesn't seem to have any power to correct this mistake. |
I am inclined to say this is a failure of the project maintainers. |
I honestly would love to hear why "It makes sense to render docstrings as comments", considering this is completely against the style guide and its entirely objective that doc-strings are NOT comments. Don't they think that if this made sense, that someone might have proposed it before? We've literally been rendering doc-strings in a different colour to comments for many years, and no one has been asking to change it. Did they not consider that that might be an indication that its that way for a reason? It has since been fully explained in the original PR as to why the change was incorrect, and yet its been completely ignored by both the original reviewers. It'd be nice to see an apology for the mistake, and then some kind of review as to how this happened and why it is taking so long to correct. |
Let's keep things community spirited 😊 I think it was a mistake, and it is indeed frustrating that this is taking so long, compared to the first change, but that is as it is. Asking for apologies, borne from frustration, feels counter productive.
I would suggest better guidelines around approving changes to existing behaviour, that aren't operational bugs. Linux kernel's mantra of "don't break userspace" comes to mind. In this case, the change did two things
What we might be able to extrapolate from that as a guideline for future approval processes, I cannot say for sure. My suggestion though would be to ask
If so (therefore an option is being introduced, rather than a bug being fixed), choose to pursue that route. If not (this may affect bug fix changes), ask
If it affects all users within a language/technology community, seek multiple maintainer involvement. My 2c, for what they're worth 🪙. I hear inflation is high these days. |
I don't think the first of these is correct? I don't have an older VSCode to hand to test with, however looking at the original PR I don't see anything there which changes how the parsing logic worked -- only changes to how the built-in themes coloured tokens of various types. (Plus some tests changes) The original PR even admits that this was already possible for individual users -- it includes a link to a StackOverflow answer written by the author on the same day they opened the PR which explains how to do it! Your other suggestions for how to introduces this sort of change in a non-breaking fashion are completely valid though and are exactly what the VSCode team have done in the past. |
@PeterJCLaw You're right. |
I would generally agree, but they didn't ask the community about the change in the first place, and then are ignoring the community when asked to fix it? It seems to me, that the very least they could do, would be to apologise. Perhaps that just me. What I do not understand, is why this issue has 11 approvals and can't be merged, but the original PR was merged with only 2 approvals? This makes no sense to me... |
|
@hediet as you have some of the context from where this issue was raised on microsoft/pylance-release#4429, are you able to provide any insight into how the community can help get this moving? |
|
i do not support changing back. At first i was "surprised" and though that i had some error with my linter or vs code configuration. But then after looking into it and understanding the problem and more importantly, the rationalle for the solution, it makes perfect sense for doc strings / block quotes to be the same colour as other comments... In fact, i always found it rather odd that block comments were a different colour to other comments. So the correction imposed is actually correcting something that had historically always been incorrecy & i applaud VS code for its straight and logical thinking on the matter. |
But doc strings are not comments, they have very different purpose. And even if some people abuse multiline strings for commenting the code, it doesn't excuse rendering them the same way. |
|
Hey everyone, we will revert for now to the old behavior we had and once @aeschli is back he can drive the discussion going forward. |
You are entitled to your opinion. But it has now been explained by many people why doc-strings are not comments, and why this conclusion is absolutely clear and objective from reading the python style guide, and understanding the design, implementation, and behaviour of Python. The vast majority of people in the previous PR and the mountain of issues that have been created around this, also agree that treating doc-strings as comments is incorrect. This is a community based project, and therefore they should yield to the prevailing opinion of the community. I do not applaud them, because they made a illogical decision, with no clear reasoning behind it or why they think it "makes sense". And they did it without consulting the community at all, leaving many confused and deeply frustrated. Now, even with a huge amount of support to revert to the previous correct behaviour, it is taking forever. |
|
it is absolutely logical for comments to all be on one colour. Multicoloured coments are a distraction from the code. I was also surprised at first, but after realising the simple rationalle, it makes perfect sense irrespective of what luddites or others with legacy code have to say. Progress is forwards and just because we were doing things incorractly in the past, does not mean we should not do them properly in the future. Single colour comments are an excellent idea. |
|
I guess the best way moving forward is to give users a choice. |
|
agreed. i would think that providing users with a choice (check-box) of multicoloured comments would keep python 100% of the universe happy. |
You're either ignoring what has been said, or you're misunderstanding the problem. The point, as has been explained very clearly numerous times now, is that doc-strings are not comments. Which is why your statements do not make much sense at all and don't have any reasonable rationale behind them. This is why almost everyone agrees the old behaviour is better. You are entitled to your opinion, as i've said, but you're making very strong statements that are founded on fundamentally incorrect idea. I would agree that allowing for a choice is absolutely fine. But the problem with the original PR was that it was forced upon us without choice, and the majority of the community disagreed with it. |
|
Lets make it abundantly clear. Anything that classify as "notes for humans to read" and "does not affect the code itself" is essentially a comment. And a doc string falls exactly into that category irrespective of what you have incorrectly asserted. Many users are quite comfortable in seeing these in the same colour as they have the same impact on the code (None). In fact, i actually enjoy the docstring being in the same colour as the rest of the comments. I like colours to be reserved for keywords and other code types that have an impact on the behaviour of the code. So "colourful comments" (including doc strings) should be an optional extra for people, like yourself, who vehimently care for such a feature. This would keep everybody happy. Less is more. |
|
Furthermore, if you can show me meaningfull cases where doc strings impact the actual workings of the code, then i will accept that they should be colourful. Otherwise they should not. |
making a different part of your sentence bold makes it clear that even by your standards, docstrings are not identical to comments. They are indeed similar and docstrings can in certain cases or definitions be considered comments, but they are not identical. Also, calling people who disagree with you "luddites" etc, is not a helpful argument... |
(I think this has also been mentioned multiple times) Docstrings, as defined in python glossary, is a language feature of python, and is treated by the interpreter differently. Specifically, Another reason I'm against your opinion is that "notes for humans to read" are too general. For languages that cannot distinguish, what you said above is okay. However python provides the infrastructure to separate "notes for users" (docstrings) and "notes for developers" (comments), and I think this alone can serve as a sufficient reason to render them differently. (Yet I do admit that maybe we should differentiate plain strings and docstrings, but this is another story which requires further discussion)
Then you can configure it yourself, just like in the current workaround. And It seems a consensus that vscode should provide a better way of allowing people to configure it. However, as a widely used tool like vscode, any change to the default behaviour that can majorly impact UX should be carefully reviewed, instead of directly enforcing the preference of some to the rest. If the community agrees with the change, then it is (even so, we should find some way to minimize UX impact). Otherwise we'd better stick to the old behaviour.
As said above, docstrings natively affect dunder attributes of specific objects, and thus you're wrong from the very beginning with the assertion that docstring "does not affect the code itself". You don't use it does not mean everyone doesn't, especially when it's a native feature. (If it's indeed useless, "docstring" will never be introduced in python) |
Introducing autodox, a package that generates documentation using type annotations and docstrings. This uses the |
|
Please revert it! This change makes the experience very bad. |
...to be formatted like comments. VSCode just undid my PR in the official release of VSCode, so put back this setting again now. Here's where they undid my PR: https://code.visualstudio.com/updates/v1_81#_pull-requests See the revert: "@lkct (Rickey K. Liang): Revert Python docstring color PR #184938". microsoft/vscode#184938 So, I have to manually force docstrings to look like comments in Python again. Here's my original PR that the revert above reverted: microsoft/vscode#182162
This is a temporary fix of #184836 and reverts PR #182162.
Motivation:
Therefore, I'm proposing to revert the change and go back to the old behaviour in the next release. The issue will not be closed by this and we should eventually find a correct solution.