[RFC][direnv-inspired] introduce export and hook commands, use in devbox shell, global and direnv #1172
Conversation
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
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
savil
force-pushed
the
savil/direnv-inspired-shellenv
branch
from
8975339 to
e24ad15
Compare
savil
force-pushed
the
savil/direnv-inspired-shellenv-2
branch
from
95ab2db to
a9c13a8
Compare
savil
force-pushed
the
savil/direnv-inspired-shellenv
branch
from
e24ad15 to
61a3bfd
Compare
savil
force-pushed
the
savil/direnv-inspired-shellenv-2
branch
3 times, most recently
from
602db68 to
5f9367c
Compare
savil
changed the title
savil
force-pushed
the
savil/direnv-inspired-shellenv-2
branch
from
5f9367c to
e23f447
Compare
savil
changed the title
loreto
approved these changes
Jun 20, 2023
| func hookCmd() *cobra.Command { | ||
| flags := hookFlags{} | ||
| cmd := &cobra.Command{ | ||
| Use: "hook [shell]", |
There was a problem hiding this comment.
Another possible name for this command is activate (it's what https://github.com/jdxcode/rtx uses)
Leave as hook, and let's get the PR in, but I wanna get John's thoughts on which one is the better name, before we remove the feature flag and start transitioning people.
There was a problem hiding this comment.
yes, agree on activate. Please see my RFC proposal in the PR summary for reasons why I like it...
savil
force-pushed
the
savil/direnv-inspired-shellenv
branch
from
61a3bfd to
751628e
Compare
savil
force-pushed
the
savil/direnv-inspired-shellenv-2
branch
from
e23f447 to
f4dc79b
Compare
savil
force-pushed
the
savil/direnv-inspired-shellenv-2
branch
from
f4dc79b to
cc2aff7
Compare
savil
added a commit
that referenced
this pull request
Jun 28, 2023
## Summary Minor followup to #1172 ## How was it tested?
1 task
savil
added a commit
that referenced
this pull request
Jun 29, 2023
…e in devbox shell, global and direnv (#1172)" (#1233) ## Summary Reverts #1172. While we feel architecturally this is a better approach, it would introduce some non-trivial risk. For the moment, bin-wrappers seem to have stabilized so we'll punt on this, and circle back to it later. ## How was it tested? compiles - [ ] testscripts should pass
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.
Summary
See #1156 for overall motivation.
In this PR:
devbox shellenv.devbox hookanddevbox export, analogous todirenv hookanddirenv export.exportis an alias of shellenv, and is hiddenhookregisters a shell prompt hook that wil calleval $(devbox export)devbox shell, we addeval $(devbox hook)to the generated shellrc file.direnv, the code is essentially unchanged except callingdevbox exportinstead ofdevbox shellenv.eval $(devbox global hook <shell>)RFC proposal to roll out:
devbox export. We can continue withdevbox shellenv.devbox hookentirely: in the generated shellrc, we can inline the code of the hook that callsdevbox export.devbox global activateto have the functionality thatdevbox global hookhas in this PR.activateis a generic word, enabling us to change its functionality under-the-hood. It would save us the dilemma we have with this PR about how to make users change their shellrc code fromdevbox global shellenvtodevbox global hook.devbox global shellenvan alias ofdevbox global activate. This would save users from needing to update their personal shellrc files.How was it tested?
Added a feature flag:
DEVBOX_FEATURE_PROMPT_HOOK=1.Did some basic testing:
devbox shelland then editeddevbox.jsonto add a new package, and saw the new package being installed prior to next prompt being displayed.devbox run -- iex -S mixworked as expected, since we can turn off the bin-wrappers.devbox global hook | sourceto my fish shell.devbox global lsworked.devbox global add <package>worked as before.Note: this PR adds the basic functionality, but the feature is not yet robust. For a full list of pending scenarios that must work, please refer to the (internal) notion doc.