Make el-patch-get use the only variant by default #67
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
I make significant use of `el-patch-validate` non-interactively in my
init file. These usages broke when I updated to the latest version of
el-patch, because an invocation that used to work such as
(el-patch-validate 'some-function 'defun 'nomsg)
was now unable to look up the patch I had just applied to
`some-function`. I believe this is happening because
`el-patch-validate` was allowing you to omit `variant`, and `nil` is
not a possible variant. From reading the code, I believe all variants
will be a cons of `(advice name)`. Therefore when `el-patch-validate`
calls `(el-patch-get 'some-function 'defun nil)`, it will never get a
result, and `el-patch-validate` will always fail.
`(nil)` *could* perhaps be called "the default variant", but I didn't
see it hard coded anywhere else in el-patch, so I wasn't about to
start here. Instead, I made `el-patch-get` (which also makes the
`variant` argument optional) use the one and only variant present for
the given name. If more than one variant is present, and the caller
didn't supply `variant`, `el-patch-get` now signals an error. I
picked `wrong-type-argument` because it was the standard error closest
in spirit to what I was trying to convey, and I didn't feel that this
case called for defining a whole new error.
haji-ali
added a commit
that referenced
this pull request
Apr 9, 2023
|
Thank you! Will go ahead and close this as I think the issue is resolved by 7dfbd37. |
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.
I make significant use of
el-patch-validatenon-interactively in my init file. These usages broke when I updated to the latest version of el-patch, because an invocation that used to work such aswas now unable to look up the patch I had just applied to
some-function. I believe this is happening becauseel-patch-validatewas allowing you to omitvariant, andnilis not a possible variant. From reading the code, I believe all variants will be a cons of(advice name). Therefore whenel-patch-validatecalls(el-patch-get 'some-function 'defun nil), it will never get a result, andel-patch-validatewill always fail.(nil)could perhaps be called "the default variant", but I didn't see it hard coded anywhere else in el-patch, so I wasn't about to start here. Instead, I madeel-patch-get(which also makes thevariantargument optional) use the one and only variant present for the given name. If more than one variant is present, and the caller didn't supplyvariant,el-patch-getnow signals an error. I pickedwrong-type-argumentbecause it was the standard error closest in spirit to what I was trying to convey, and I didn't feel that this case called for defining a whole new error.