-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
nix repl: Print which variables are just loaded #11406
base: master
Are you sure you want to change the base?
Conversation
06bb355
to
667a643
Compare
e9c815f
to
9db7cce
Compare
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.
This is very nice! I find myself reading through all symbols in the tab completion sometimes, and that's just bad tbh.
src/libcmd/repl.cc
Outdated
@@ -470,6 +472,10 @@ ProcessLineResult NixRepl::processLine(std::string line) | |||
loadFlake(arg); | |||
} | |||
|
|||
else if (command == ":ll" || command == ":last-flake") { |
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.
What's the idea behind two l
s?
Doesn't this also apply to variables loaded with :l <file>
?
Maybe it should be :last-scope
or :show-scope
.
:ls
is tempting, but we might want to use that for :b
+ readDirectory
, so I'm not sure about that.
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.
ll = last loaded
. The name :show-scope
would be misleading I believe, since we're not actually showing the global scope, just the items that were most recently loaded. :last-scope
or :ls
might be a bit too close to the program ls
and can cause confusion.
:ll
applies to anything that loads an attr set into global scope, that is :a
, :l
, :lf
, and repl cmdline args.
tests/functional/repl.sh
Outdated
- 0 | ||
- 1 | ||
- 2 | ||
- 3 | ||
- 4 | ||
- 5 | ||
- 6 | ||
- 7 | ||
- 8 | ||
- 9 | ||
... And 3 other variables, use :ll to see these |
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.
Looking at this, it's not using rows efficiently. Maybe long lists could be broken into columns instead, if that interests you?
I don't think we have a function for rendering columns yet.
We do have getWindowSize().second
for the terminal width.
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.
Now doing comma-separated. We should ideally escape keys containing ,
and quotes and so on. Do we have an already existing function for this purpose? I could not find one.
Hi @kstrafe, how's it going? Do you plan to continue to work on this, or do you prefer that someone else takes over? |
When we run `nix repl nixpkgs` we get "Added 6 variables". This is not useful as it doesn't tell us which variables the flake has exported to our global repl scope. This patch prints the name of each variable that was just loaded. We currently cap printing to 10 variables in order to avoid excessive prints. NixOS#11404
Invoking `:ll` will start a pager with all variables which have just been loaded by `:lf`, `:l`, or by a flake provided to `nix repl` as an argument. NixOS#11404
Hey, I've just come back to this. I've added your suggestions and fixed up the tests. They aren't very easy to debug. I've added dumps to files and
Is there a more practical way? It's quite slow. Anyhow, we now show all loaded variables on a single line. I have not handled escaping (for keys containing commas and spaces), but I believe that's quite rare so probably not very important to handle. Is there any way to test the pager as well? I'd like to test |
@roberth mentioning you since your profile mentioned it. |
When we run
nix repl nixpkgs
we get "Added 6 variables". This is not useful as it doesn't tell us which variables the flake has exported to our global repl scope.This patch prints the name of each variable that was just loaded. We
currently cap printing to 10 variables in order to avoid excessive
prints.
This PR also introduces
:ll
for listing all previously loaded variables in a pager. This list is overwritten by the next load.Github issue: 11404
This PR also introduces
:ll
which starts a pager containing all loaded variables.