Load ~/.jq as a library #1830
Merged
Load ~/.jq as a library #1830
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
|
Tests fail, FYI. |
muhmuhten
force-pushed
the
init_as_lib
branch
from
9862438
to
f5b2241
Compare
|
Well okay, looks like I introduced a(?) memory leak, but it's a works-on-my-machine so this space might get a bit noisy with pushes as I try to suss that out with the CI. |
"Module path must be a string" is not a useful error message when the reason the module path isn't a string is because the string it was got replaced with an invalid with an error message for some other reason. Also fixes a few memory leaks on early exits.
Only the second and subsequent path components were being checked, which I guess is theoretically security-relevant. There's no apparent point to reconstructing the path after splitting it by adding /s back in, either.
The case isn't actually possible afaict.
A library marked is imported if found, but silently skipped if missing. This is the desired semantic for the auto-include at ~/.jq
Remove the special code which loads ~/.jq in builtin.c, and instead glue an optional include which points to the same file onto the main program in linker.c. Fixes a minor bug where errors in ~/.jq would be labelled <builtin>.
muhmuhten
force-pushed
the
init_as_lib
branch
from
f63370a
to
893dd2d
Compare
|
Tests are passing now, or at least the ones that normally pass anyway. I squashed bugfixes into their corresponding commits; sorry for the extra noise, though. |
|
Yeah, I've been meaning to figure out what's up with the tests that fail all the time... |
|
Thanks!!! |
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.
There are no performance improvements in this patchset, unfortunately.
It's just an attempt at cleaning up
builtins_bindto make a better springboard for working out the details of #1826 without having ~/.jq guts in the same function, when it can better be loaded as a library that just happens to have the same name and location.There is a small incompatibility in the resolution order of shadowed names, but that's already buggy IMO (#1828) so I don't think it matters.
(muhmuhten@0702990 is a slightly more truthful history of this changeset.)