[nothing to see here] lib: Add functionInfo#194992
Closed
infinisil wants to merge 1 commit intoNixOS:masterfrom
Closed
[nothing to see here] lib: Add functionInfo#194992infinisil wants to merge 1 commit intoNixOS:masterfrom
infinisil wants to merge 1 commit intoNixOS:masterfrom
Conversation
e78c79f to
5f70c50
Compare
roberth
reviewed
Oct 7, 2022
| function = getFunction f; | ||
| xmlString = builtins.toXML function; | ||
| builtinString = "<?xml version='1.0' encoding='utf-8'?>\n<expr>\n <unevaluated />\n</expr>\n"; | ||
| argumentRegex = "<\\?xml version='1.0' encoding='utf-8'\\?>\n<expr>\n <function>\n <varpat name=\"(.*)\" />\n </function>\n</expr>\n"; |
Member
|
Interesting PR... :D |
vkryachko
added a commit
to vkryachko/nixpkgs
that referenced
this pull request
Sep 9, 2023
Having used `callPackage` a bit, it seems a bit odd and inconsistent that modules require `...` in their signatures as opposed to figuring out the signature and passing only what's requested, like `callPackage` does. Now I don't know if there is an actual reason for why it is important for modules to require `...`, but I could not find any and upon a brief chat with @infinisil, it seemed like there might not be a reason. So if you know of a reason, please let me know. Worth noting that, as currently implemented, it's is a breaking change, namely modules that use at-pattern arg capture don't work (`{ ... }@args`, args used to contain all available module args, but is now empty, see newly added failing test for details). I don't have any data on how common it is to write modules with at-pattern capturing, so would like to get some guidance on how important it is to preserve this behavior. It's possible to make it a non-breaking change by detecting `...` and/or `@args` with something like NixOS#194992 or rather with a new builtin like was proposed in NixOS#7317, and passing all available arguments if detected. Would like to get your thoughts on the idea to see if there are any strong objections to pursuing this further.
This was referenced Sep 9, 2023
This file contains hidden or 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
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.
Description of changes
Adds a powerful
functionInfofunction, giving you more information about your functions!PS: Don't look at the implementation
PPS: This PR is only half-serious, motivation came from #194514 (comment), where this PR would actually have a good use case (cc @roberth)
Things done