Skip to content

Conversation

@erezsh
Copy link
Member

@erezsh erezsh commented Sep 21, 2021

No description provided.

@erezsh erezsh requested a review from MegaIng September 21, 2021 17:46
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2021

Codecov Report

Merging #1006 (911a129) into master (6514314) will decrease coverage by 0.91%.
The diff coverage is 8.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1006      +/-   ##
==========================================
- Coverage   87.66%   86.74%   -0.92%     
==========================================
  Files          49       50       +1     
  Lines        6973     7055      +82     
==========================================
+ Hits         6113     6120       +7     
- Misses        860      935      +75     
Flag Coverage Δ
unittests 86.74% <8.53%> (-0.92%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lark/tree_templates.py 0.00% <0.00%> (ø)
lark/indenter.py 88.60% <100.00%> (+1.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6514314...911a129. Read the comment docs.

Copy link
Member

@MegaIng MegaIng left a comment

Choose a reason for hiding this comment

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

There are a few more things that might be useful, but that should be done in the future. (e.g. some kind of repeated vars).

# 1. Define a Python parser that also accepts template vars in the code (in the form of $var)
#
TEMPLATED_PYTHON = r"""
%import python (single_input, file_input, eval_input, atom, var, stmt, expr, testlist_star_expr, _NEWLINE, _INDENT, _DEDENT, COMMENT, NAME)
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a perfect usecase for an %include statement.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that too

Copy link
Member Author

Choose a reason for hiding this comment

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

You didn't submit a PR for this, right?

Copy link
Member

Choose a reason for hiding this comment

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

It is part of #998, but that is not a finished implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm okay. Anyway, we can change it to %include when it's ready.

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

from python_parser import PythonIndenter

# Official Python grammar by Lark
python_parser3 = Lark.open_from_package('lark', 'python.lark', ['grammars'],
Copy link
Member

Choose a reason for hiding this comment

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

Do we need .lark?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to manually add the extension .lark

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't work without it, so I'll go with yes

@erezsh erezsh merged commit c13b56a into master Oct 18, 2021
@erezsh erezsh deleted the tree_templates branch October 18, 2021 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants