Skip to content

Added todict() method to Command#67

Open
Seemone wants to merge 9 commits into
tonioo:masterfrom
Seemone:master
Open

Added todict() method to Command#67
Seemone wants to merge 9 commits into
tonioo:masterfrom
Seemone:master

Conversation

@Seemone

@Seemone Seemone commented Aug 10, 2018

Copy link
Copy Markdown

Adds a method to represent the sieve filters as dicts

usage example:

    jlist=[]
    if p.parse(filters):
        for r in p.result:
            jlist.append(r.todict())
        print(json.dumps(jlist),file=outfile)

@tonioo tonioo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please make your PR compatible with PEP8.

Comment thread sievelib/commands.py Outdated

"""

j=OrderedDict()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you respect PEP8 please?

Comment thread sievelib/commands.py Outdated
j=OrderedDict()
comms=[]

if isinstance(self,TestCommand):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Same here.

Comment thread sievelib/commands.py Outdated
if self.children:
j.update({"children": []})
for child in self.children:
#target.write(("*" * (indentlevel+10))+'{"'+argname+'": "'+str(arg)+'"},\n')

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you remove this comment?

@codecov

codecov Bot commented Aug 17, 2018

Copy link
Copy Markdown

Codecov Report

Merging #67 into master will decrease coverage by 2.12%.
The diff coverage is 15.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage   84.07%   81.94%   -2.13%     
==========================================
  Files          10       10              
  Lines        1683     1734      +51     
==========================================
+ Hits         1415     1421       +6     
- Misses        268      313      +45
Impacted Files Coverage Δ
sievelib/parser.py 86.41% <100%> (ø) ⬆️
sievelib/commands.py 85.87% <13.46%> (-9.75%) ⬇️

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 9d28ce4...284bd51. Read the comment docs.

@Seemone

Seemone commented Aug 17, 2018

Copy link
Copy Markdown
Author

Modified and checked against http://pep8online.com/
The only outstanding issue is a line too long in the original scope (out of scope of this PR) that it's too long because of an inline comment.

@tonioo

tonioo commented Aug 17, 2018

Copy link
Copy Markdown
Owner

Can you add some unit tests please?

BTW, I think you should be interested by this method: https://github.com/tonioo/sievelib/blob/master/sievelib/commands.py#L269.

@Seemone

Seemone commented Aug 17, 2018

Copy link
Copy Markdown
Author

I have no idea how to do it, sorry.

@Seemone

Seemone commented Aug 17, 2018

Copy link
Copy Markdown
Author

But it would be a good idea, since I already discovered a bug :)

@Seemone

Seemone commented Aug 21, 2018

Copy link
Copy Markdown
Author

I'm going to add an attribute name mapping layer that will change the keys of the arguments

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.

2 participants