Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve YANG parsing especially for comments and spacing #244

Closed
wants to merge 4 commits into from

Conversation

wlupton
Copy link
Contributor

@wlupton wlupton commented Jul 14, 2016

This is the first of three pull requests, each of which builds on the previous one. The first step is to improve the YANG parsing and YANG output format so that well-written YANG will be unchanged by being passed through pyang. The second step is to add a new yang-edit output format that can edit the YANG in various ways. One advantage of this approach (over, for example, YIN and XSLT) is that the full pyang parse tree is available and therefore quite sophisticated editing operations are easily implemented. A YIN/XSLT approach would in any case need the YANG parsing improvements.

The pull requests (referring to them by their branch names) are:

  • yang-parse: (this one) The main emphasis is on comment handling (including allowing comments to precede the module/submodule statement) and line-handling (so the output can retain things like blank lines and trailing comments). The implementation approach was chosen to minimise change from the current implementation but a better approach would probably be to completely re-think how comments are handled (they are currently handled as statements but this doesn't really work, because they should be syntactically equivalent to white space).
  • yang-output: (includes the yang-parse commits) The main changes are supporting a new --yang-keep-blank-lines option and pretty-printing quoted strings (including splitting them into multiple lines where necessary in order to avoid exceeding designated maximum line lengths).
  • yang-edit: (includes the yang-output commits) Defines a new yang-edit output format that inherits from the yang output format. Currently supports various specific --yang-edit-xxx options with various heuristics for how the replacement values are specified. Could support generic XPath-based editing operations.

@coveralls
Copy link

coveralls commented Jul 14, 2016

Coverage Status

Coverage increased (+1.4%) to 71.199% when pulling e7971ed on BroadbandForum:yang-parse into f46eb62 on mbj4668:master.

Conflicts:
	pyang/yang_parser.py
@coveralls
Copy link

Coverage Status

Coverage increased (+7.1%) to 37.899% when pulling cbe75e3 on BroadbandForum:yang-parse into 5190231 on mbj4668:master.

@coveralls
Copy link

coveralls commented Jul 14, 2017

Coverage Status

Coverage increased (+0.02%) to 31.73% when pulling 19125c6 on BroadbandForum:yang-parse into 0066d30 on mbj4668:master.

@wlupton
Copy link
Contributor Author

wlupton commented Jul 14, 2017

@mbj4668 I suggest ignoring (rejecting) this PR. This is because the yang-output one includes all of its commits.

@coveralls
Copy link

coveralls commented Jul 14, 2017

Coverage Status

Coverage increased (+0.02%) to 31.73% when pulling 394315a on BroadbandForum:yang-parse into 0066d30 on mbj4668:master.

@wlupton
Copy link
Contributor Author

wlupton commented Nov 3, 2018

I've decided to give up on this one and am closing it. Comment processing has been improved since I created it (over two years ago!). You still can't put comments before or after the module/submodule statement, but this is a theoretical rather than a practical problem. Also, comment locations aren't always preserved (e.g. inline comments go onto the next line, possibly preceded by a blank line) but again this isn't really a huge issue!

I have rescued the "smart quoting" part of this code and contributed it and some other minor changes via a new PR #436.

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.

3 participants