Skip to content
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

RFC: Redesign config structure #5

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions text/0005-config_design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
- Feature Name: Redesign of nushells config structure
- Start Date: (fill me in with today's date, 2021-03-18)
- RFC PR: [nushell/rfcs#0005](https://github.com/nushell/rfcs/pull/0005)

# Summary
[summary]: #summary

This RFC shall provide the basis for a structured discussion, how the nushell config could be improved.

# Motivation
[motivation]: #motivation
Important: This document is written as if, https://github.com/nushell/nushell/pull/3041 would already be accepted and merged. The PR is currently in review.


Nushell evolved over time and features got added. The current config format may need to be adapted to better represent nushells capabilities to new users.

The config structure is mostly well done, but some enhancements I can think of:
1. Every `[...]` table has a name without config, but there is `[color_config]` which I think should be renamed to `theme` (better consistency, better naming).
LhKipp marked this conversation as resolved.
Show resolved Hide resolved
2. We could add a table for aliases. Example:
```toml
[aliases]
vim = "vscode" # The new kids on the block won't even notice
emacs = "echo Hit exit-meta-alt-control-shift at the same time to enter emacs" # Hehe does nothing :)
```
3. We could give $PATH ($nu.path) his own section. Example:
```toml
[path]
defaults = ["/usr/bin", "/home/sweet/home"]
vpn_client = "/opt/vpn_client/bin"
cargo = "/home/leo/.cargo"
```
4. Some settings are not put inside a table `[...]`. Those could at least be grouped under `[general]`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agreed about [general]. I'm definitely for more orgainization.

5. With `.nu-env` and the "global" config under `$HOME/.config/nu/config.toml` being merged, we can think of `autoenv` as a local config (a config local to a directory). Therefore we could rename `.nu-env` to `.nu/config.toml`. (We may need to adapt the `autoenv` commands accordingly)
- Rename `autoenv trust` to `package trust` (see 9)
- Rename `autoenv untrust` to `package untrust` (see 9)
6. We have `startup` and `on_exit`. We could rename `startup` to `on_enter` (Better naming, better consistency (`enter`/`exit` command)).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also think about adding a on_enter.nu and on_exit.nu file right next to the config.toml. on_enter and on_exit config values don't scale past 1 line of code. Files (I think) would suit most users better.

Copy link

@fdncred fdncred Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me, I've been looking at the fish shell source code recently. I noticed that most, if not all, of their built in commands have cpp code but they also all have *.fish commands too. i.e. there's cd an cd.fish. I was thinking that this could be an approach we take with nushell in our stdlib.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds cool. Is it something we need to consider when redesigning the config (and how configs should work)?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably a broader conversation about stdlib and built-in commands in general. so, we can probably drop the discussion about it since it's not specifically related to config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. But I generally agree with your opinion. It would also allow us to prototype stuff faster :).

7. `startup` for most people I guess (?), contains mostly `alias`'es and `def`'s. Aliases are short and can be put under one table `[aliases]`, `def`'s might be long and best put into individual files. We could add a directory `defs` for `def`'s.
Example: File `$HOME/.config/nu/defs/cd.nu`
```nu
def cd [path] {ls $path; ^cd $path}
```
8. `textview`, `line-editor` and `theme (aka color_config)` are not reconfigured when loading local config files. I don't think this has priority, but would be cool to have.
```diff
cd clown_repo:poop: ; ls
-────┬───────────┬──────┬─────────────┬──────────────
+ # │ name │ type │ size │ modified
!────┼───────────┼──────┼─────────────┼──────────────
# 0 │ Android │ Dir │ 4,096 │ 2 months ago
- 1 │ Desktop │ Dir │ 4,096 │ 1 week ago
+ 2 │ Documents │ Dir │ 4,096 │ 1 month ago
! 3 │ Downloads │ Dir │ 4,096 │ 1 week ago
# 4 │ Music │ Dir │ 4,096 │ 2 months ago
@ 5 │ Pictures │ Dir │ 4,096 │ 1 month ago
+ 6 │ Postman │ Dir │ 4,096 │ 2 months ago
! 7 │ Public │ Dir │ 4,096 │ 3 months ago
# 8 │ README.md │ File │ 984 │ 4 days ago
- 9 │ Templates │ Dir │ 4,096 │ 2 months ago
+ 10 │ Videos │ Dir │ 4,096 │ 2 weeks ago
! 11 │ bin │ Dir │ 4,096 │ 4 days ago
@ 12 │ nushell │ Dir │ 4,096 │ 4 days ago
- 13 │ pictures │ Dir │ 4,096 │ 3 months ago
+ 14 │ repos │ Dir │ 4,096 │ 1 hour ago
! 15 │ tags │ File │ 100,126,720 │ 1 month ago
#────┴───────────┴──────┴─────────────┴──────────────
```
9. (Note: This is very similar to neovims `package` idea.)
With adding a `defs` directory, we already make the step from a config file, to a config directory. We can treat such a directory as a `package`. We could make sharing of `[theme]`, `defs` pretty easy by loading all packages under a common path (`$nu.packpath`). Example nu startup sequence:
- Nu is loading my global config / `package` ($HOME/.config/nu) first.
- Nu now loads all `packages` in `$nu.packpath`
- `$nu.packpath` contains a package with a `defs` directory, full of wrapper around git commands. Those are loaded.
- `$nu.packpath` also contains a package with a config.toml file having a `[theme]` table. Now nu sets the theme from it.

The nice thing about it is:
- `package`'s can be version controlled (simple git repo) and therefore be easily made available online.
- Writing a `package manager` is now the same as writing a wrapper around git.

There are some arguments speaking in favor of this approach:
1. Relativly easy
2. Local configs already brought the possibility to load (and unload) multiple `config.toml` files. This is nothing "completly" new.
3. Proof of concept is done by neovim.

This needs more design, but could be a general consideration when restructuring the config.

10. The `config` command may needs redesign. It does currently not take local configs into account. I think the following adoptions should be done:
- Remove `config clear` (I personally think its unnecessary)
- Remove `config path` (see `package list`)
- Rename `config remove` to `config rm` (Better naming)
- Add `package list` => List of tables of currently loaded configs/packages with path
- Add `package load` => Load a config/package
- Add `package unload` => Unload a config/package
- Add `package init (path)` (Initialize package structure under path, so users don't have to look it up all the time)
- Allow passing of --local to `config rm <key>` (Remove key from last loaded local config), `config set/set_into <key>` (Set key/value into last loaded local config), `package list` (Only show local configs).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my one wish for a config command set update is that we move to toml_edit so that comments are respected and maintained. today, with our current config setting tools, comments are removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. The current config code makes a lot of assumption. It will need some substantial rework, especially after accepting this RFC.

# Drawbacks
[drawbacks]: #drawbacks

Everything done to the structure of the config is a breaking change for most users.

# Prior art
[prior-art]: #prior-art

I did a very little research what other shells are doing:
- Zsh:
- Package Manager: https://github.com/ohmyzsh/ohmyzsh
- https://github.com/ohmyzsh/ohmyzsh/issues/6788 (TLDR: Install oh-my-zsh, add a .zsh file under a specific path). I am not sure how easy one can add someone else package, if he didn't contribute it to the (kinda central?) plugin repository under ohmyzsh.
- Doesn't look to me, that they have some plugin/package structure.
- Fish:
- Package Manager: https://github.com/oh-my-fish/oh-my-fish
- Example Package: https://github.com/oh-my-fish/plugin-expand
- Seems like fish has a well defined config directory structure. Maybe we could steal some ideas from them :)