Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Integrate MagicPython in Atom #109

Closed
1st1 opened this issue Oct 19, 2015 · 17 comments
Closed

Integrate MagicPython in Atom #109

1st1 opened this issue Oct 19, 2015 · 17 comments

Comments

@1st1
Copy link

1st1 commented Oct 19, 2015

Hi,

There are many open issues (#55 or #39 are especially hard to fix) for this package that are non-issues for the https://github.com/MagicStack/MagicPython syntax highlighter. We've spend quite a bit of time designing a new python highlighter from scratch, along with writing a huge corpus of unit tests. Should we consider integrating it in Atom by default?

@50Wliu
Copy link
Contributor

50Wliu commented Oct 19, 2015

I have a fix open for both of those issues right now - are there any other benefits that MagicPython would bring (other than more unit tests, which are always nice)? Would you consider PRing the improvements from Magic Python over to this repo?

@1st1
Copy link
Author

1st1 commented Oct 19, 2015

I have a fix open for both of those issues right now - are there any other benefits that MagicPython would bring [..]

Some benefits:

  • We parse regex syntax on a very granular level -- (?<foo>) has different scope from (?=) etc, and we highlight comments properly, try r'''foo (?#sfd''' or simply r'''foo #sfd''')
  • It's not possible to break highlighting using an invalid regex in an r'..', r"..", r'''..''' or , r"""..""" strings.
  • we support binary strings b".."
  • async/await and all other new syntax of Python up to 3.5
  • function parameter annotation support is very robust
  • we parse docstrings differently from normal strings, that allows us to highlight them differently (and it's great, as they do serve a different purpose), as well as avoid highlighting raw docstrings as regexps.
  • we highlight keywords as keywords when they used as keyword arguments or parameters, so that you know that you're doing something wrong when you type a(async=1) or def a(yield=1)
  • there are many more smaller details and nits, please take a look at our tests.

[..] (other than more unit tests, which are always nice)?

Even if you decide against integrating MagicPython, I'd suggest you to evaluate using our syntaxdev package. It simplifies writing unit-tests for syntaxes greatly.

Would you consider PRing the improvements from Magic Python over to this repo?

Since MagicPython was written from scratch, it's not easy to incorporate its improvements. I'd simply consider reusing its 'cson' file in this package, or integrating MagicPython as a submodule repo.

Here are some screenshots, built-in Python syntax on the left, MagicPython on the right (some examples are a bit crazy, they come from our unit-tests):

screen shot 2015-10-19 at 5 24 54 pm
screen shot 2015-10-19 at 3 52 43 pm
screen shot 2015-10-19 at 3 52 57 pm
screen shot 2015-10-19 at 3 53 39 pm
screen shot 2015-10-19 at 3 54 09 pm
screen shot 2015-10-19 at 3 54 33 pm
screen shot 2015-10-19 at 3 55 56 pm
screen shot 2015-10-19 at 3 57 10 pm
screen shot 2015-10-19 at 3 58 24 pm
screen shot 2015-10-19 at 3 59 05 pm
screen shot 2015-10-19 at 3 59 46 pm
screen shot 2015-10-19 at 4 26 25 pm
screen shot 2015-10-19 at 4 27 03 pm
screen shot 2015-10-19 at 4 30 41 pm
screen shot 2015-10-19 at 4 34 46 pm
screen shot 2015-10-19 at 4 36 04 pm
screen shot 2015-10-19 at 4 36 26 pm
screen shot 2015-10-19 at 4 36 58 pm
screen shot 2015-10-19 at 4 38 30 pm
screen shot 2015-10-19 at 5 06 18 pm

@Dowwie
Copy link

Dowwie commented Oct 20, 2015

+1 MagicPython adoption -- a complete rebuild lead to greater enhancements

@1st1
Copy link
Author

1st1 commented Feb 9, 2016

It looks like github/linguist now uses MagicPython to highlight Python code on GitHub. Maybe we should use it in Atom editor by default too? MP is super stable at this point.

@amirrustam
Copy link

+1 for MagicPython adoption as a default. I imagine many Python devs will install it right away when setting up Atom.

@p-e-w
Copy link

p-e-w commented May 15, 2016

I also support this move. MagicPython is the superior Python highlighter, probably the best Python highlighter available today for any editor. Even for very simple code that does not use fancy features like type annotations, MagicPython often produces subjectively "better" results, an example being highlighting from and import in different colors.

Probably the most compelling argument to switch is that MagicPython also supports Sublime and VSCode, so it is going to benefit from a wide community exposure, which would soon make it the better system anyway if it wasn't already.

@nicktimko
Copy link
Contributor

Given the increasing amount of unsupported syntax (see: all pull requests) and how catastrophically it can break the TextMate-ported highlighter, this needs to occur. Most of the outstanding PRs are bandaids on a severed limb.

@1st1
Copy link
Author

1st1 commented May 18, 2016

I'll make a PR in a few days.

@50Wliu
Copy link
Contributor

50Wliu commented May 18, 2016

@1st1 First of all, sorry for the late response (I've been meaning to reply to your comment on the other issue for a few days now, but it keeps slipping my mind 😞). Can you please hold off creating a PR for now? We haven't worked out the specifics of how we're going to migrate to MagicPython yet, but once we do I'll be sure to let you know!

@1st1
Copy link
Author

1st1 commented May 18, 2016

@50Wliu Sure, NP.

FWIW my plan was to make MagicPython a submodule of language-python. Ideally, we should keep the MagicPython as upstream, as we want to keep maintaining it, add new language features once Python 3.6 is out etc.

If you just copy MagicPython grammar to language-python, that will inevitably result in maintenance overhead for all of us.

@1st1
Copy link
Author

1st1 commented May 18, 2016

/cc @elprans @vpetrovykh

@1st1
Copy link
Author

1st1 commented Aug 25, 2016

FWIW VSCode now includes MagicPython by default.

@iolsen
Copy link

iolsen commented Dec 2, 2016

I spoke in Slack with @ambv about swapping out language-python for MagicPython in the next beta release.

This will be primarily (entirely, perhaps?) a change in atom/atom. If someone wants to start the PR there one of us GitHubbers (or perhaps @50Wliu) can get involved in the next couple of weeks. In particular making sure it gets properly packaged on the various CIs, you’re likely to need help.

@1st1
Copy link
Author

1st1 commented Dec 2, 2016

I can make a PR sometime next week.

@ambv
Copy link
Contributor

ambv commented Feb 22, 2017

Yury, ping ;)

@1st1
Copy link
Author

1st1 commented Feb 22, 2017

@ambv @iolsen

Sorry for the delay. I asked my colleague @elprans to help us with this. He's an active Atom user and maintains a Gentoo package for it. He'll make a PR in a couple of days.

@50Wliu
Copy link
Contributor

50Wliu commented Feb 6, 2018

We have decided not to migrate to MagicPython. Please see atom/atom#13877 (comment) for the rationale.

@50Wliu 50Wliu closed this as completed Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants