xpath: add support for indexed xpath #112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeanseb6wind
force-pushed
the
add_index_support_to_xpath_set
branch
3 times, most recently
from
527a545 to
89847e0
Compare
rjarry
requested changes
Mar 26, 2024
libyang/xpath.py
Outdated
| break | ||
| j += 1 | ||
| key_value = xpath[i:j] | ||
| keys.append((KEY_INDEX, key_value)) |
There was a problem hiding this comment.
I am a bit concerned that the keys list can now contain tuples that are not only strings. It would be less disruptive if the key name would be an empty string in this case.
You can check that below without much hassle.
libyang/xpath.py
Outdated
| @@ -134,6 +147,12 @@ def _list_find_key_index(keys: List[Tuple[str, str]], lst: List) -> int: | |||
| if py_to_yang(elem) == keys[0][1]: | |||
| return i | |||
|
|
|||
| elif keys[0][0] == KEY_INDEX: | |||
There was a problem hiding this comment.
Please change to:
elif keys[0][0] == "":
tests/test_xpath.py
Outdated
| @@ -101,7 +102,7 @@ def test_xpath_set(self): | |||
| {"name": "eth3", "mtu": 1000}, | |||
| ], | |||
| "lst2": ["a", "b", "c"], | |||
| "lstnum": [1, 10, 20, 30, 40, 100], | |||
| "lstnum": [1, 10, 20, 30, 33, 100], | |||
There was a problem hiding this comment.
Could you add more unit tests. This is a rather risky change. Thanks.
This patch adds support for indexed xpath to xpath_set. In the case of a leaf-list, it is now possible to apply xpath_set in this way: xpath_set(d, "/lstnum[5]", 33, after="4") This will replace the 5th element of the leaf-list with the new value 33. This feature is important because libyang can return this type of xpath when a callback is called with sr_module_change_subscribe from sysrepo. The tests are updated accordingly. Signed-off-by: Jean-Sébastien Bevilacqua <[email protected]>
jeanseb6wind
force-pushed
the
add_index_support_to_xpath_set
branch
from
89847e0 to
1347f5f
Compare
|
@rjarry I updated it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for indexed xpath to xpath_set. In the case of a leaf-list, it is now possible to apply xpath_set in this way:
xpath_set(d, "/lstnum[5]", 33, after="4")
This will replace the 5th element of the leaf-list with the new value 33.
This feature is important because libyang can return this type of xpath when a callback is called with sr_module_change_subscribe from sysrepo.
The tests are updated accordingly.