Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Fix/misc annotations - #112

Merged
juanjux merged 6 commits into
bblfsh:masterfrom
juanjux:fix/misc_annotations
Oct 2, 2017
Merged

juanjux merged 6 commits into
bblfsh:masterfrom
juanjux:fix/misc_annotations

Conversation

@juanjux

@juanjux juanjux commented Sep 28, 2017 •

Copy link
Copy Markdown
Contributor
  • Remove Function and Declaration from arguments (they were wrong when in a call
    and superfluous when under a real declaration node).
  • Remove Call from arguments (the parent already have it).
  • Add the Module role to the root node.
  • Remove duplicated roles for comparisons inside comprehensions.

Fixes #97 (and suggestions/findings from other issues).

Will be merged after the Python-driver refactor changes are.

@juanjux juanjux self-assigned this Sep 28, 2017
@juanjux
juanjux requested a review from abeaumont September 28, 2017 16:29

@abeaumont abeaumont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think leaving either Function or Call along with Argument is useful, since it allows to differentiate between formal and actual parameters directly.

That's how the java driver is annotated at least, so in either case it should be made consistent among drivers.

@juanjux

juanjux commented Sep 29, 2017

Copy link
Copy Markdown
Contributor Author

Yes, thinking about it, and considering the dynamic nature of Python, something like:

b = 2
def myfunc(a=otherfunc(b)): 
    pass

Could be much easier to parse if a and b are clearly marked as formal/actual parameters.

@abeaumont abeaumont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm, only Call or Function is used in java driver, but I like this more. The problem is that it's hard to filter function declarations in this new way. We need to think how to improve this, but it's out of the scope of this PR.

@juanjux juanjux changed the title [Not for Merge] Fix/misc annotations Fix/misc annotations Oct 2, 2017
Juanjo Alvarez added 6 commits October 2, 2017 18:44
- Remove Function and Declaration from arguments (they were wrong when in a call
  and superfluous when under a real declaration node).
- Remove Call from arguments (the parent already have it).
- Add the Module role to the root node.
- Remove duplicated roles for comparisons inside comprehensions.
@juanjux
juanjux merged commit c968ad1 into bblfsh:master Oct 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants