Add support for parsing dotted names into tags#57
Add support for parsing dotted names into tags#57kaushikcfd wants to merge 13 commits intoinducer:mainfrom
Conversation
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Thanks for working on this. Some initial high-level feedback below.
kaushikcfd
force-pushed
the
parse_tag
branch
2 times, most recently
from
44f0ead to
c2f9057
Compare
- limit line columns length to 85 - install lark-parser for pylint,docs to resolve modules
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Thanks! Some initial comments below.
| rm pytools/log.py | ||
|
|
||
| export EXTRA_INSTALL="numpy" | ||
| export EXTRA_INSTALL="numpy lark-parser" |
There was a problem hiding this comment.
Could you push a sister MR to Gitlab (add the link to the PR description) to ensure that passes, too?
There was a problem hiding this comment.
pytools/tag.py
Outdated
| TAG_GRAMMAR = """ | ||
| tag: tag_class "(" params ")" -> map_tag_from_python_class | ||
| | SHORTCUT -> map_tag_from_shortcut | ||
|
|
||
| params: -> map_empty_args_params | ||
| | args -> map_args_only_params | ||
| | kwargs -> map_kwargs_only_params | ||
| | args "," kwargs -> map_args_kwargs_params | ||
|
|
||
| ?kwargs: kwarg | ||
| | kwargs "," kwarg -> map_kwargs | ||
|
|
||
| args: arg -> map_singleton_args | ||
| | args "," arg -> map_args | ||
|
|
||
| kwarg: name "=" arg -> map_kwarg | ||
|
|
||
| ?arg: tag | ||
| | INT -> map_int | ||
| | ESCAPED_STRING -> map_string | ||
|
|
||
| tag_class: module "." name -> map_tag_class | ||
|
|
||
| module: name -> map_top_level_module | ||
| | module "." name -> map_nested_module | ||
|
|
||
| name: CNAME -> map_name | ||
| SHORTCUT: "." ("_"|LETTER) ("_"|LETTER|DIGIT|".")* | ||
|
|
||
| %import common.INT | ||
| %import common.ESCAPED_STRING | ||
| %import common.DIGIT | ||
| %import common.LETTER | ||
| %import common.CNAME | ||
| %import common.WS | ||
| %ignore WS | ||
| """ |
There was a problem hiding this comment.
Looked over the grammar, looks OK. (Please leave this for me to resolve at final review.)
|
Probably I should rewrite this as |
|
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
I actually don't think so. We'll be able to reuse this as we use more lark. Sorry I've been slow to review. Otherwise ready for another look from your end? |
2 participants
No description provided.