search

kimgr / asn1ate

A Python library for translating ASN.1 into other forms.
Other
69 stars 41 forks source link

Code modifications as requested #37

Closed vanrein closed 7 years ago

vanrein commented 8 years ago

Hello Kim,

This is a new Pull Request that I wish to propose to you. It processes your suggestions, to some of which I also made a small comment. I only did not remove the space in "key: value" forms because it made the text very difficult to read, IMHO. I hope you don't mind that.

This PR comes from the "donate-back" branch, which differs from my fork master only in the Makefile that I added just for asn2quickder, and that I assume is not of interest to you.

Thanks, -Rick

kimgr commented 8 years ago

Thank you! I lost track of this during my vacation, hoping to get around to reviewing it again soon.

kimgr commented 8 years ago

Please find some initial comments in line. I think this needs tests, especially the new imports/exports stuff, but I'm also anxious to see a simple test suite for your code generator, so I don't accidentally break it.

kimgr commented 7 years ago

Sorry for not getting to this sooner. I've given this some thought, and it occurred to me that there's no real point to merging the two projects.

asn1ate is designed to be a parser/codegen library -- the pyasn1 generator is just a first example that happens to be generally useful. Internally, we have another code generator that produces pyasn1 and some other, more high-level definitions from the same ASN.1 source.

If you design asn2quickder to just import asn1ate as a third-party, I think things will become much easier -- you don't have to succumb to my Python style whims :-), and I don't have to worry about breaking the asn2quickder backend when I make changes. You can reintegrate and test with asn1ate whenever I publish a new release.

Let me know if that makes sense to you. I can definitely help you get started on a separate project.

vanrein commented 7 years ago

Hello Kim,

asn1ate is designed to be a parser/codegen library -- the pyasn1 generator is just a first example that happens to be generally useful. Internally, we have another code generator that produces pyasn1 and some other, more high-level definitions from the same ASN.1 source.

OK, and I viewed asn1ate as an application and thus wanted to add my alternative to the package.

If you design asn2quickder to just import asn1ate as a third-party, I think things will become much easier -- you don't have to succumb to my Python style whims :-)

No real pain there, and that's good because I'd still have to dance by your steps when adding ASN.1 parser pieces such as what I did for Imports / Exports and what I suggested for CLASS / WITH SYNTAX.

and I don't have to worry about breaking the asn2quickder backend when I make changes. You can reintegrate and test with asn1ate whenever I publish a new release.

So asn1ate becomes an external library. Yes, that makes sense to me.

That still leaves the library changes that I put in the Pull Request. I suppose you will want me to adapt it to include only those, but leave out asn2quickder.py then, right?

Let me know if that makes sense to you. I can definitely help you get started on a separate project.

It makes perfect sense to me. I do hope the parser changes are still up for inclusion though. New PR with just those?

-Rick

kimgr commented 7 years ago

I do hope the parser changes are still up for inclusion though. New PR with just those?

Yes, please!