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

New Parser #80

Merged
merged 23 commits into from
Jan 24, 2015
Merged

New Parser #80

merged 23 commits into from
Jan 24, 2015

Conversation

hit9
Copy link
Contributor

@hit9 hit9 commented Jan 23, 2015

Main changes: (All changes are parser part.)

  1. include_dirs to include_dir, parser will try to find all child thrifts files inside this directory during a parsing progress. (Non-Backward-Compatible)
  2. Fix referenced types and values, the features were not supported or not well supported in old parser. ( Add support for default enum values that reference the original enum. #69 "const map<EnumType, ?>" fails to parse #75)
  3. Ply will parse thrift to python module directly, skipping the original to-json-data-parsing.
  4. All constants and ttypes (structs/unions/exceptions/..) are using the same one namespace now, each will be parsed as a member of the parsed thrift module.
  5. Constants (or as default field value) can reference to constants or named enum values now.
  6. The new parser is more strict on types. Definitions like const i32 i = 1.5 will be rejected now.
  7. Including child thrift files perfects well now, all child thrifts will be parsed during a single parsing progress. (via a thrift stack)
  8. Strict fields checking on defining a struct instance. struct Person {1: required string name}; const Person tom = {} will be rejected now.
  9. Namespaces are ignored.
  10. Same with old load, we can enable cache on parsing (default enabled)
  11. Cycling including checking (or dead including checking). Inluding case like a->b->c->a will be rejected.

The changes above contains 1 non-backward-compat change.

The parser makes thriftpy behaves closer to apache thrift.

@maralla will merge this pull request.

@hit9 hit9 mentioned this pull request Jan 23, 2015
@hit9
Copy link
Contributor Author

hit9 commented Jan 23, 2015

Dont merge now, I will add some test cases.

@tmehlinger
Copy link
Contributor

These are excellent changes. Really looking forward to having this mainlined. 😄



def _cast_string(v):
assert isinstance(v, str)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should test for basestring on Python 2.6/2.7, else it will fail on unicode strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thriftpy always read thrift files as strings, not unicode strings.

@hit9
Copy link
Contributor Author

hit9 commented Jan 24, 2015

Can be merged now.

Better to upgrade thriftpy after merge this pr.

maralla added a commit that referenced this pull request Jan 24, 2015
@maralla maralla merged commit 07db77b into Thriftpy:develop Jan 24, 2015
@hit9 hit9 deleted the new-parser branch January 24, 2015 11:20
@lxyu
Copy link
Contributor

lxyu commented Jan 27, 2015

Excellent! 😄

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.

4 participants