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

Api shape unification #19

Merged
merged 9 commits into from
Apr 27, 2020
Merged

Conversation

sbcgua
Copy link
Collaborator

@sbcgua sbcgua commented Apr 26, 2020

OK, so check_object supports unified API shape from abaplint/abaplint-sci-server#28 (abaplint/abaplint-sci-server#27 for reference)

This is a dirty one. I'd like to separate API response processing to another class, but later. This approach works for now.

As a dependency improved JSON parser. Looks like it may become a separate lib :) Added sub_section which bytes branch of json to a separate reader - useful to remove API wrapper.

P.S. some comments to json code from the commits:

Rethink of split_path: Split is the method to create a path+name that can be found in table.
So the previous logic, that treated ending / and a delimiter does not make sense. I had a feeling that it was wrong, now I know. So now: /a/b/ results in '/a/' + 'b', and / results in '' + '' which is existing root element !
This however allows accessing attrs with '/' at the end of the path - like '/value/'. Maybe this is not good. But if needed this can be guarded in get_item instead of split_path

P.P.S would be very nice to have drilldown to issues ... :) But I have no experience with SCI internals ...

sbcgua added 2 commits April 26, 2020 21:14
Rethink of split_path: Split is the method to create a path+name that can be found in table.
So the previous logic, that treated ending / and a delimiter does not make sense. I had a feeling that it was wrong, now I know. So now: /a/b/ results in '/a/' + 'b', and / results in '' + '' which is existing root element !
This however allows acessing attrs with '/' at the end of the path - like '/value/'. Maybe this is not good. But if needed this can be guarded in `get_item` instead of `split_path`
@sbcgua
Copy link
Collaborator Author

sbcgua commented Apr 26, 2020

... totally confused with the linter issue ... I'd propose to increase line length to 130. ?

P.S. offtopic, carring to new lines the if conditions seems to me the least obvious abaplint rule. It is never intuitive how to do indentation there. I tried 0, 2, 3 chars in this case - all failed. This is abaplint topic actually, but I think something has to be done to this. Maybe change the message. Or make this rule more tolerant.

@larshp larshp merged commit 14ed1de into abaplint:master Apr 27, 2020
@sbcgua
Copy link
Collaborator Author

sbcgua commented Apr 27, 2020

So it's 4 chars after all ... ok :))))

@sbcgua sbcgua deleted the api-shape-unification branch July 4, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants