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.
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
Pyk: KCFG stores aliases for nodes #2647
Pyk: KCFG stores aliases for nodes #2647
Changes from 34 commits
852f1e5
a9a42e5
9d8711f
7b7ffbb
f82f8ea
48665f3
08b8678
30a20ab
e902270
e9a87f2
498b2d2
fdba4e8
790eec3
55774df
5c6e1a1
dfc9ec5
546ddba
cb84a94
19746a6
32b0ee1
774f5d6
9737103
aa44ddc
e69db22
c826baa
e81fed7
d94a7f6
7140a92
9336d8e
4d3459d
8c4e860
4af47ef
5099939
89a4c52
6af15d9
3dac0e3
9da584c
d1b1b64
69071e4
2f026b9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call this
_resolve_hash
, and keep its old behavior.Then
_resolve_or_none
can be implemented as follows.id_like
is a hash, then delegate to_resolve_hash
, and raise an exception if there are multiple results.@
, drop the first character.As a result,
@
is optional, and can be used to enforce strings to be resolved as aliases (as something starting with@
is never a hash). So for exampleabcd
is resolved as a hash@abcd
is resolved as aliasabcd
hello
is resolved as aliashello
@hello
is resolved as aliashello
Optionally, we can also refine
_resolve
to give a more specific error message if resolution fails:id_like
is a hash:Unknown node hash: ...
Unknown alias: ...
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the refactoring. But wasn't the whole point of adding the
@
to avoid confusion between the two?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that if
id_like
is not a valid hash, we can omit the@
, because we know we have an alias. But this is just a small usability improvement.In any case, the input should be validated to be either a hash or an alias (if
@
is optional, the latter does not have to be checked).