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

[python] Replace TextMate for Python syntax highlighting #2208

Closed
nchammas opened this issue Jan 24, 2016 · 20 comments
Closed

[python] Replace TextMate for Python syntax highlighting #2208

nchammas opened this issue Jan 24, 2016 · 20 comments
Assignees
Labels
feature-request Request for new features or functionality languages-basic Basic language support issues
Milestone

Comments

@nchammas
Copy link

VS Code's syntax highlighting for Python is broken in a few ways.

VS Code currently uses TextMate's Python bundle for syntax highlighting. The problem is that this project seems to be unmaintained. The last commit was from mid-2014.

We should replace this with a project that is more up-to-date and actively maintained. PythonImproved seems to fit the bill, though I haven't used it yet. I've asked the maintainers of that project about compatibility with VS Code.

If anyone has other suggestions for a better tokenizer for Python, please speak up.

@aeschli aeschli added the languages-basic Basic language support issues label Jan 27, 2016
@aeschli aeschli added this to the Feb 2016 milestone Jan 27, 2016
@aeschli
Copy link
Contributor

aeschli commented Feb 10, 2016

There's also #2867 suggesting to take MagicPython.
Did give this a try? (There's already a MagicPython extension)

@aeschli aeschli changed the title Replace TextMate for Python syntax highlighting [python] Replace TextMate for Python syntax highlighting Feb 10, 2016
@nchammas
Copy link
Author

Just gave it a try. Looks good to me!

@aeschli aeschli modified the milestones: March 2016, Feb 2016 Feb 22, 2016
@MattDMo
Copy link

MattDMo commented Feb 22, 2016

I'm the maintainer of Python Improved, and I have no issues at all with your using it to supplement or replace the TextMate syntax definition in VSCode. One issue I've found with Magic Python is that a number of the scopes are rather different than the TM/Sublime Text originals, so switching to that could cause issues with color schemes that aren't compatible with MP. Those of us working on PI with me have tried to maintain compatibility with the original scopes as much as possible (or, at least, as much as makes sense), so it should work with a broader array of color schemes. I've tailored my Neon Color Scheme to work with all the new scopes in PI, in case you want to include that as well.

Let me know what you think, or if I can help in any way. Alternatively, I can add PI and Neon to the VSCode store if you think that would be a better option.

@nchammas
Copy link
Author

Perhaps @aeschli can offer advice on a path to take (I'm just a random user), but I would guess a solid first step would be to make PythonImproved available as a VS Code extension and then take it from there.

@aeschli
Copy link
Contributor

aeschli commented Feb 23, 2016

@MattDMo Thanks Matt for your comment. Yes, making Python Improved available as a VS Code extension would be a great first step so more people can try it out.
Let me know if you need help with the extension. We have a yeoman generator that helps you setting up an extension that contributes a grammar. Check out https://code.visualstudio.com/docs/tools/yocode.
To replace an existing grammar use the same language id ('python') and additionally add

    "extensionDependencies": [
        "vscode.python"
    ]

to your package.json

@aeschli
Copy link
Contributor

aeschli commented Apr 21, 2016

There has been new work in the TextMate's Python bundle. So I've updated the grammar to the latest.

I'm still open to update to any other grammar if the community has a clear preference. But then, as said, thanks to the extension model it's possible to add grammars as extensions and each user is free to choose what to use.

@DonJayamanne
Copy link
Contributor

@MattDMo , have you had any luck in building an extension for VS Code. I'm certain there would plenty of people within the community and the VS Code team willing to help you with this.

@aeschli aeschli assigned DonJayamanne and unassigned aeschli Jul 14, 2016
@DonJayamanne
Copy link
Contributor

@MattDMo , @aeschli , I'm am in the process of creating a VS Code extension using the code from Python Improved.

The issue #2936 doesn't appear in Magic Python, however when using the tmLanguage file from PythonImproved, I can see that the issue still exists.
Issue #7688 doesn't seem to be a non-issue in both Magic Python and Python Improved.
However, #7688 can be replicated when using PythonImproved on TextMate!

Those are the only two issues I could find.
Anyways, I haven't published the extension yet and it can be found here

@1st1
Copy link

1st1 commented Jul 28, 2016

@DonJayamanne As a maintainer of MagicPython I'd strongly recommend you to go with it. Simply because we embrace TDD, MP is super stable and we can add new features very quickly. It's also used by GitHub to highlight Python sources. We have 100s of complex automated unittests, something that no other syntax highlighter for any other language does (for tmLanguage format anyways).

Some additional features of MP (I just went through our unittests and checked which ones break standard highlighter or PI):

  • It parses docstrings differently from string literals, allowing you to highlight them in a different way and making sure raw strings aren't highlighted as regexps.
  • Highlights cases like illegal use of literals:
some. True
a = True
  • Highlights self and cls for methods:
class F:
    def __init__(self, a, b=1):
        self.a = a
  • Can highlight type annotations (GH's color schema doesn't support them yet, but try it with MP and Chromodynamics color schema):
x = None # type: List[str, a]
y = None # type: Dict[int, Any] # int
  • Highlights doctests:
r'''Module docstring

    Some text followed by code sample:
    >>> for a in foo(2, b=1,
    ...                 c=3):
    ...   print(a)
    0
    1
'''
  • Correctly handles parameter annotations (try that in PythonImproved):
def some_func(a:
                 lambda x=None:
                    {key: val
                        for key, val in
                            (x if x is not None else [])
                    }=42):
  • Correctly handles invalid line continuations:
1 + \ sdgfsdf
def foo(): pass
  • Superb support of both old and new style string formatting:
a = 'qqq{0:{width}{base}}www'
a = 'qqq{0:$20}www'
a = 'qqq{0}www'
  • Unicode is, of course, fully supported (as in PI, I believe):
class Üa(Êa):
    \nŃ'
    @æaœ
    def ŌÏŒĘ(self, Ú=1):
        print('превед 你好')
        return Ù
    你好 = lambda: 你好
    def 你好(): pass

There are much more small, but handy (sometimes obscure) features in MP, but I don't want to list too many examples.

@1st1
Copy link

1st1 commented Jul 28, 2016

Also, MP is already used by >31000 VSCode users: https://marketplace.visualstudio.com/items?itemName=magicstack.MagicPython

@nchammas
Copy link
Author

As the person who reported this issue, I can also add that I'm a happy MagicPython user too. 👍

@MattDMo
Copy link

MattDMo commented Jul 28, 2016

I have no issues with using MagicPython, if that's the way you want to go. PythonImproved is just a side project in my free time (I'm actually a research scientist in biotech in real life), and honestly I just don't have the time at the moment to support some of the features that MP does already. If there's anything in PI that can be useful, please feel free to grab it (just attribute me in some way). Hopefully I'll eventually get around to releasing PI for VSCode, but work is definitely my top priority at the moment, so I don't know when that will be.

Sorry I can't spend more time on it, but since PI isn't directly related to my primary job functions at work, the company won't support my time spent on an open source project. Please do keep me in the loop, though, and I'll try to answer any concerns that may arise.

@DonJayamanne
Copy link
Contributor

@aeschli , my plan is to update the Python tmlanguage files (later today) with the ones from MagicPython:

  • Will I need to create any test plans? I don't see the need for this? However, a sanity check wouldn't hurt
  • Will I (or someone else) need to update the August iteration plan?

FYI - as this is a simple update to a tmlanguage file, I won't be creating a PR.

@1st1
Copy link

1st1 commented Aug 17, 2016

FYI - as this is a simple update to a tmlanguage file, I won't be creating a PR.

One quick note just in case: please make sure that the name key is set to Python (not MagicPython) in the replaced tmLanguage. We'd like to have an option for our users to manually install MagicPython and not have any potential conflicts.

Also, would it be possible to publicly state that VSCode uses MagicPython and to direct highlighter related issues to our tracker?

@DonJayamanne
Copy link
Contributor

@1st1

make sure that the name key is set to Python

sure thing

publicly state that VSCode uses MagicPython

No sure how this would work, any idea how this would work @aeschli? However redirecting highlighter related issues to Magic Python tracker shouldn't be an issue.

@aeschli
Copy link
Contributor

aeschli commented Aug 17, 2016

  • Can you point me to the github repo where the magic python grammar file is located? I'll then file a legal approval request and we can update OSSReadme.json which will add a reference to Magic Python and it's license to our thirdpartynotices file.
  • We currently have no mechanism to tell users to directly file issues against to the Magic python repo, but we will move the requests if they are filed against us after verifying it's not a bug in our text-mate parser or theme infrastructure.
  • A test plan item would make sense. We should test the grammar with all our built-in themes. Just create a issue that explains what to test and add the 'test-item' label and set milestone to August.
  • When updating, also verify the highlighting of the test file at update the syntax highlighting tests (run with ./scripts/test-integration, accept changes if you have verified that the new result makes sense).
    Feel free to add more syntax highlighting tests.
  • I started adding update scripts as to make it easier to update grammar files. Check out
    https://github.com/Microsoft/vscode/blob/master/extensions/scss/package.json#L7 as an example. It's invoked with npm run update-grammar

@DonJayamanne
Copy link
Contributor

@aeschli

@aeschli
Copy link
Contributor

aeschli commented Aug 17, 2016

I initiated the OSSRequest. You can proceed and change the OSSReadme file to

// ATTENTION - THIS DIRECTORY CONTAINS THIRD PARTY OPEN SOURCE MATERIALS:
[{
    "name": "MagicStack/MagicPython",
    "version": "0.0.0",
    "license": "MIT",
    "repositoryURL": "https://github.com/MagicStack/MagicPython"
}]

Let me know one you committed the changes, then I'll trigger the script that updates the ThirdPartyNotices.

@DonJayamanne
Copy link
Contributor

@aeschli , I will submit a PR later today.
With regards to the update-grammar script:

  • Is this something that needs to be run manually or is this part of the build process? If it is manual, then that's fine
  • The update-grammar script converts the tmLanguage file to json. However looking at existing code, we don't do this for the Regular Expressions (JavaScript).tmLanguage file.
    Would the same apply to the python regular expressions tmLanguage file.

I.e. the final result would be as follows (one json and other tmLanguage):

"grammars": [{
            "language": "python",
            "scopeName": "source.python",
            "path": "./syntaxes/Python.tmLanguage.json"
        },{
            "scopeName": "source.regexp.python",
            "path": "./syntaxes/Regular Expressions (Python).tmLanguage"
        }]

@aeschli
Copy link
Contributor

aeschli commented Aug 18, 2016

@DonJayamanne Thanks for looking into the grammar update scripts.

It’s a manual step. We want to update at a controlled time (e.g. not right before a release so that we get a chance to receive bug reports in time) and also do some sanity testing before pushing a grammar update.

I have just started adopting the scripts. I will add the script and migrate from using .json to .plist/.tmLanguage when there is a need to update the grammar.
Care is required as in some grammars we made manual changes, in anticipation of a fix or from a pull request, that I need to port over. So best is to check the commit history of the grammar to see what happened. Ideally such additions always have a test with them, like https://github.com/Microsoft/vscode/blob/master/extensions/javascript/test/colorize-fixtures/test6916.js

Yes, if Regular Expressions (Python).tmLanguage is still in use, then I’d suggest to update it as well.
The script takes multiple parameters if both grammars are from the same repo (see https://github.com/Microsoft/vscode/blob/master/extensions/cpp/package.json#L7).
Otherwise use && to run multiple commands in one script.

aeschli added a commit that referenced this issue Aug 22, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
daviddossett added a commit that referenced this issue Oct 24, 2023
Alex0007 pushed a commit to Alex0007/vscode that referenced this issue Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality languages-basic Basic language support issues
Projects
None yet
Development

No branches or pull requests

5 participants