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

Quotes to wrap XpathExpression should be kept as what it is after serialization #118

Closed
huyuwen opened this issue May 17, 2018 · 9 comments
Assignees

Comments

@huyuwen
Copy link
Contributor

huyuwen commented May 17, 2018

We recently found that all Quotes which used to wrap XpathExpression will be removed after serialization. For example, original,
leaf lf2 {
type uint8;
must '. <= ../lf1';
}
After serialization,
leaf lf2 {
type uint8;
must . <= ../lf1;
}
We double checked with RFC 7950, actually, it is valid according to the 'must' and 'when' definition.
But we still propose to wrap XpathExpression with Quotes, or at least keep as what it is. There are a few reasons,

  1. The example in RFC 7950, (ex, must statement and when statement) all wrap with quotes. That means according to the spec, they recommend to have the quotes.
  2. If there are WS in XpathExpression, Quotes can avoid some parse errors. Actually, pyang doesn't accept 'must' or 'when' expression without quotes if there are WS in expression. Pyang will report it as 'error: unterminated statement definition for keyword "must", looking at...'
  3. XpathExpression with quotes has better readability
@JanKoehnlein JanKoehnlein self-assigned this May 17, 2018
@JanKoehnlein
Copy link
Contributor

Could you elaborate on the serialization usecase? Do you read in an existing model with an existing XPath or do you create a new UnparsedXpath? Is it in the SerializationIntegrationTest?

@huyuwen
Copy link
Contributor Author

huyuwen commented May 17, 2018 via email

@JanKoehnlein
Copy link
Contributor

Depends on eclipse/xtext-core#747

JanKoehnlein added a commit that referenced this issue May 22, 2018
Signed-off-by: Jan Koehnlein <jan.koehnlein@typefox.io>
JanKoehnlein added a commit that referenced this issue May 23, 2018
GH-118 Preserve quotes on serialization
@JanKoehnlein
Copy link
Contributor

Fixed

@huyuwen
Copy link
Contributor Author

huyuwen commented May 29, 2018

This issue has to be re-opened.
We found if there has '+' in the path string, serialization will be wrong.
Let's still take the xpath-serialize.yang as example.
if we define another leaf node in list (lb),
leaf lfb2 { type leafref { path "/ytest:c1" + "/ytest:l1"; } }
Then the path in lfb2 will be serialized to
{ path "/ytest:c1 "/ytest:l1"; }

JanKoehnlein added a commit that referenced this issue May 30, 2018
GH-118 reopen issue: Take string concatenations into account
@JanKoehnlein
Copy link
Contributor

Fixed

@huyuwen
Copy link
Contributor Author

huyuwen commented May 31, 2018

Unfortuantely, we found some other strange behaviors.

  1. '/' position is the end of a segament. there will be an unnecessary space ' ' append into that segament. See output example.
    Input: leaf lfb2 { type leafref { path "/ytest:c1/" + "ytest:l1"; } }
    Output: leaf lfb2 { type leafref { path "/ytest:c1/ " + "ytest:l1"; } }
  2. segament break at ':', whatever it is at the end or at the beginning of a segament, there will be unnecessary space ' ' append into the segament
    Input: leaf lfb2 { type leafref { path "/ytest:" + "c1/ytest" + ":l1"; } }
    Output: leaf lfb2 { type leafref { path "/ytest: " + "c1/ytest " + ":l1"; } }

The space will make xpath totally wrong.

@JanKoehnlein JanKoehnlein reopened this May 31, 2018
JanKoehnlein added a commit that referenced this issue May 31, 2018
Signed-off-by: Jan Koehnlein <jan.koehnlein@typefox.io>
@JanKoehnlein
Copy link
Contributor

Never mind. I've added the missing case and more tests.

JanKoehnlein added a commit that referenced this issue May 31, 2018
GH-118 also consider XpathNameTest elements
@JanKoehnlein
Copy link
Contributor

Hopefully fixed now. Don't hesitate to reopen if you encounter further pecularities.

huyuwen added a commit to huyuwen/yang-lsp that referenced this issue Jun 1, 2018
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

No branches or pull requests

2 participants