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

Conversation

@juanjux
Copy link
Contributor

@juanjux juanjux commented Sep 6, 2017

Originally from: /pull/16 moved to my branch to continue the work.

All suggestions in the original PR have been implemented on this branch.

Other changes:

  • Added error control which turns an ugly crash into a nice traceback (+ unittest).
  • Migrated to libuast 0.2.0.
  • Migrated to Google's C++ coding style
  • Changed autorship to source{d} in the setup.py, updated email.

@juanjux juanjux changed the title Add libuast [WIP] Add libuast Sep 6, 2017
@juanjux juanjux mentioned this pull request Sep 6, 2017
@juanjux juanjux changed the title [WIP] Add libuast Add libuast, upgrade libuast API, bugfixing Sep 7, 2017
@juanjux juanjux requested a review from abeaumont September 7, 2017 12:27
@juanjux
Copy link
Contributor Author

juanjux commented Sep 7, 2017

Looks like the new tests if failing with Python 3.4 which makes CI fails, I'll take a look.

- Added error control which turns an ugly crash into a nice traceback (+ unittest).
- Migrated to `libuast` 0.2.0.
- Migrated to Google's C++ coding style
- Changed autorship to source{d} in the setup.py, updated email.

Whitespace

Fixed test
bblfsh/pyuast.c Outdated
PyMODINIT_FUNC PyInit_pyuast(void)
{
NodeIface iface = {
.InternalType = GetInternalType,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use InternalType, etc. instead of GetInternalType, etc. so be consistent with code in both libuast, its examples, client-go, etc.

bblfsh/pyuast.c Outdated
}

static PyMethodDef extension_methods[] = {
{"find", PyFind, METH_VARARGS, "Find a node in the UAST"},
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rename find to filter for consistency (I changed the name since we're not looking for a node, we're filtering the nodes in the tree that match some predicate.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're not breaking backward compatibility 👍

@juanjux
Copy link
Contributor Author

juanjux commented Sep 7, 2017

Symbols renamed.

@juanjux juanjux merged commit 8fb23b8 into bblfsh:master Sep 7, 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.

3 participants