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

Add support for function annotations#95

Closed
berdario wants to merge 4 commits intoatom:masterfrom
berdario:master
Closed

Add support for function annotations#95
berdario wants to merge 4 commits intoatom:masterfrom
berdario:master

Conversation

@berdario
Copy link
Copy Markdown
Contributor

This thing is not quite working
I get a new support.function.magic.python apparently from nowhere

If someone can fix this and complete the PR for me that'd be nice, or also explain how the scopes names are pulled together

I tried to have a look at this, but it doesn't to deal with technical stuff

@berdario
Copy link
Copy Markdown
Contributor Author

As pointed out in the #68

There's a work that should've have these fixes already implemented... no Idea why a PR hasn't been opened (or has it been opened and then rejected)?

https://github.com/MatthewNewland/language-python3

I tried to have a look at it, but it tries to have 2 parallel grammars for Python and Python2, and it's not quick to understand the intention behind the changes

@berdario
Copy link
Copy Markdown
Contributor Author

berdario commented Oct 2, 2015

I just realized (I was sleepy when I wrote this PR, and I probably only cared for the simplest case) that, it'd need to handle arbitrary expressions in the annotations, so if someone wants to build upon this, please keep it in mind.

Basically, the following is valid Python code:

def f(a: lambda: 1) -> lambda: 1: pass

I guess this might be easier to do if language-python was built on a parser combinators library

@pchaigno
Copy link
Copy Markdown
Contributor

pchaigno commented Oct 2, 2015

Isn't this a duplicate of #88?

@refi64
Copy link
Copy Markdown

refi64 commented Oct 9, 2015

Any update on this? Trying to view Python sources on GitHub with this bug still there is starting to make my eyes die.

@thomasjo
Copy link
Copy Markdown
Contributor

@berdario The specs are currently failing.

@berdario
Copy link
Copy Markdown
Contributor Author

@thomasjo I know: I wrote

This thing is not quite working

The problem is with the scope names, and for that reason we'd need some documentation on how they are put together... maybe someone more familiar with Atom can point out where to look?

@thomasjo
Copy link
Copy Markdown
Contributor

The problem is with the scope names, and for that reason we'd need some documentation on how they are put together.

Problem is that there currently is no real guidance or documentation for this. Atom simply inherited grammars from TextMate. There is some level of standardization, but not uniformly so. A start is https://manual.macromates.com/en/language_grammars.html#naming_conventions.

A little while ago @MaximSokolov started an issue regarding the lack of a proper naming standard: atom/atom#8430. You might want to check that out as well for some guidance. For now, the best bet is to follow the TextMate guidelines, and asking for feedback on your proposed scope names. /cc @atom/feedback

@berdario
Copy link
Copy Markdown
Contributor Author

Thank you

I had another look at the error that I got: a 'support.function.magic.python' is missing, even though the other test cases that expect it still receive it, and on top of that I haven't modified the code that deals with entity.name.function.python

I guess I could try to ask on freenode #atom?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f is just entity.name.function. Remove 'support.function.magic.python'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, gotcha.... __init__ is parsed as a 'support.function.magic.python'

It was unexpected for me that it'd be tokenized differently

Thank you

@refi64
Copy link
Copy Markdown

refi64 commented Oct 13, 2015

Forgive me for the ignorance, but does this work with more complex function annotations like List[Tuple[int, Optional[str]]] or ('a', 'b', None)?

@berdario
Copy link
Copy Markdown
Contributor Author

@kirbyfan64 No, not yet

I won't have time to work on this very soon, if you want to have a stab at it please don't wait for me :)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python uses punctuation.separator.valuepair.dictionary for : inside dictionaries. So scope here should be punctuation.separator.valuepair

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm, but this is inside a definition...
we can go with punctuation.separator obviously, no strong opinion either way and not sure which one would work out better

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is : separates key from value (a is a key, int is a value. Correct me if I am wrong)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but this is not a dictionary... this is more like a triple than a pair:

def f(a:int=1): pass

a is the key, and both 1 and int are the associated values.

For tokenizing purposes, this might be splitting hairs... I'm more concerned with the fact that information about being "inside a definition" might be lost.

If punctuation.separator.valuepair would lead to better syntax highlighting I'll surely pick that

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more concerned with the fact that information about being "inside a definition" might be lost.

Do you mean inside a function definition? meta.function already append this information.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, I don't have any more doubts

@berdario
Copy link
Copy Markdown
Contributor Author

I got a non-solution:

I stopped using language-python and I moved to MagicPython

@sadovnychyi
Copy link
Copy Markdown

@berdario thanks for this. I would replace this package with magicpython. It's so much better.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants