t3rn0
New algorithm inspired by original code with inline comments in fields
Wow. That's huge:) Could you also write some tests showing what has changed? It will just be much easier to understand
I tested new and old algorithms on examples below:
Example 1
code = """# start comment
if (False # inside comment
and True): # condition comment
# if comment
a = 5
# before else comment
else: # false comment
# else comment
a = 6 # inline comment
# end comment"""
Output 1
Module(
body=[
Comment(
value='# start comment',
inline=False,
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=15),
If(
test=BoolOp(
op=And(),
values=[
Constant(
value=False,
comment=Comment(
value='# inside comment',
inline=True,
lineno=2,
col_offset=10,
end_lineno=2,
end_col_offset=26),
lineno=2,
col_offset=4,
end_lineno=2,
end_col_offset=9),
Constant(
value=True,
comment=Comment(
value='# condition comment',
inline=True,
lineno=3,
col_offset=15,
end_lineno=3,
end_col_offset=34),
lineno=3,
col_offset=8,
end_lineno=3,
end_col_offset=12)],
lineno=2,
col_offset=4,
end_lineno=3,
end_col_offset=12),
body=[
Comment(
value='# if comment',
inline=False,
lineno=4,
col_offset=4,
end_lineno=4,
end_col_offset=16),
Assign(
targets=[
Name(
id='a',
ctx=Store(),
lineno=5,
col_offset=4,
end_lineno=5,
end_col_offset=5)],
value=Constant(
value=5,
lineno=5,
col_offset=8,
end_lineno=5,
end_col_offset=9),
lineno=5,
col_offset=4,
end_lineno=5,
end_col_offset=9),
Comment(
value='# before else comment',
inline=False,
lineno=6,
col_offset=0,
end_lineno=6,
end_col_offset=21)],
orelse=[
Comment(
value='# false comment',
inline=False,
lineno=7,
col_offset=6,
end_lineno=7,
end_col_offset=21),
Comment(
value='# else comment',
inline=False,
lineno=8,
col_offset=4,
end_lineno=8,
end_col_offset=18),
Assign(
targets=[
Name(
id='a',
ctx=Store(),
lineno=9,
col_offset=4,
end_lineno=9,
end_col_offset=5)],
value=Constant(
value=6,
lineno=9,
col_offset=8,
end_lineno=9,
end_col_offset=9),
comment=Comment(
value='# inline comment',
inline=True,
lineno=9,
col_offset=10,
end_lineno=9,
end_col_offset=26),
lineno=9,
col_offset=4,
end_lineno=9,
end_col_offset=9)],
lineno=2,
col_offset=0,
end_lineno=9,
end_col_offset=9),
Comment(
value='# end comment',
inline=False,
lineno=10,
col_offset=0,
end_lineno=10,
end_col_offset=13)],
type_ignores=[])
Example 2
code = """source = {
# c1
a: 1,
b: 2, # c2
}"""
Output 2
Module(
body=[
Assign(
targets=[
Name(
id='source',
ctx=Store(),
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=6)],
value=Dict(
keys=[
Comment(
value='# c1',
inline=False,
lineno=2,
col_offset=4,
end_lineno=2,
end_col_offset=8),
Name(
id='a',
ctx=Load(),
lineno=3,
col_offset=4,
end_lineno=3,
end_col_offset=5),
Name(
comment=Comment(
value='# c2',
inline=True,
lineno=4,
col_offset=11,
end_lineno=4,
end_col_offset=15),
id='b',
ctx=Load(),
lineno=4,
col_offset=4,
end_lineno=4,
end_col_offset=5)],
values=[
Constant(
value=1,
lineno=3,
col_offset=7,
end_lineno=3,
end_col_offset=8),
Constant(
value=2,
lineno=4,
col_offset=7,
end_lineno=4,
end_col_offset=8)],
lineno=1,
col_offset=9,
end_lineno=5,
end_col_offset=1),
lineno=1,
col_offset=0,
end_lineno=5,
end_col_offset=1)],
type_ignores=[])
@t3rn0 Hello, I think kapulkin has made the requested changes. Maybe you could re-review this PR? Thank you!
Yes, I made an update with parsing bug fix. So now the code is quite stable. I use it on a daily basis.
@margual56, thank you for pointing about the update.
I believe it's still missing actual executable test cases in test_parse.py and test_unparse.py.
Also, since this proposal breaks the API of the library I think it should at least comes with the appropriate changes to the README.md file explaining the changes.
Sorry for such a late response.
This change seems broken to me. Example: there's no _Unparser definition which is used in unparse function (L241), and unparsing comments and putting them to their initial place is essential for #13.
+ there are no new test cases that would "freeze" the new features/fixes. Without it I see no reason to change anything.
I've made several attempts to work with this PR and the issue, but so far without success.
@t3rn0 thank you for the review. You are right about _Unparser, this is part of outdated code, I will remove it.
Indeed the code had also a few issues. During intensive code usage I found them and fixed few month ago on my machine. I updated the pull request now. Sorry for late update.
I agree, that future work on tests is needed to complete pull-request. Any hep here is appreciated. I will try to add tests from my side too.
Here is another piece of code that could be of interest: https://github.com/fluiddyn/transonic/blob/12aa834f9efc3c1b4c59aa3e04b2fb80bb00be15/src/transonic/analyses/extast.py#L118
Here is another interesting piece of code : https://github.com/pyccel/pyccel/blob/devel/pyccel/parser/extend_tree.py