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

Support for printing and parsing bare json values for sysrepo-gnmi #2154

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

irfanHaslanded
Copy link
Contributor

  • Support for parsing JSON bare leaf values

    Add support for parsing bare values for top-level (from the
    parser's point of view) leaf nodes, using a new LYD_OPT_BARETOPLEAF
    flag. The parsing of leaf nodes is too tied in with the allocation of
    a new node and parsing of the node name and namespace to easily
    trigger it from that case, so instead the case is check and handled
    explicitly in the top-level parser loop.

  • Support for printing JSON bare values

    Add support for printing top-level leaf nodes as bare values, which is
    triggered through the use of a new LYD_BARETOPLEAF flag.

Add support for printing top-level leaf nodes as bare values, which is
triggered through the use of a new LYD_BARETOPLEAF flag.
Add support for parsing bare values for top-level (from the
parser's point of view) leaf nodes, using a new LYD_OPT_BARETOPLEAF
flag. The parsing of leaf nodes is too tied in with the allocation of
a new node and parsing of the node name and namespace to easily
trigger it from that case, so instead the case is check and handled
explicitly in the top-level parser loop.
Copy link
Member

@michalvasko michalvasko left a comment

Choose a reason for hiding this comment

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

The parser is quite convoluted as it is so I am against adding new flags and special cases into it. Why not write a new function(s) for the functionality you want? It seems quite simple and the functions would then likely be as well.

@jktjkt
Copy link
Contributor

jktjkt commented Jan 15, 2024

Neither the commit message nor the PR description explains why this is needed. How is it related to YANG or any of its serialization formats? What is the use case, i.e., the problem that this PR solves for you? What you're doing and why you need that?

Also, the parsing code has no documentation, and the one added test works with a simple string. IMHO that doesn't really show why this is needed.

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