Codecov Report

Merging #8415 (662056e) into main (8d8304c) will decrease coverage by 0.07%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##             main    #8415      +/-   ##
==========================================
- Coverage   68.99%   68.93%   -0.07%     
==========================================
  Files         641      635       -6     
  Lines      102470   102041     -429     
==========================================
- Hits        70703    70341     -362     
+ Misses      31767    31700      -67     

@kubouch
as in #8406, maybe this PR should be marked with the std-library tag as well? 😋

It may be good to look at the tests here and make sure we have similar tests on the stdlib side.

It may be good to look at the tests here and make sure we have similar tests on the stdlib side.

💯

i can file an issue to list these tests once this PR is merged?

Can we hold off on merging this until we have an easy way for user to install the standard library? I'm working on a manual install that should be ready real soon now, then we could document a fairly smooth transition for users.

do not worry, this will stay a DRAFT until that time has come 😉

and it's mentionned at the very top in the ":warning: Warning" 😋

I have a tiny bit of hesitancy because we're essentially replacing the shells commands with nearly identical clones but we're changing all the names. I'm wondering if we should either name the dirs commands to match or make aliases to help shells users get up to speed without spending time figuring out what is broken? Interested in other's thoughts.

i do not have a strong preference 🤔 😋

• i feel the "shell" commands are primarily used in an interactive REPL, so people that use them would only have to change muscle memory / use custom aliases, unlikely fix scripts
• we can also fallback to the "legacy" names / ship "standard" aliases for backwards compatibility

what do we do here in the end @fdncred?
i do not use these commands, so i'm fine with either way 😇

i'd vote to land it after the next release in a coupld days if we can get synonym aliases for defs for backwards compatibility for enter, p, n, g. this probably also needs a git merge main since it's a bit old now.

i'd vote to land it after the next release in a coupld days

let's go for this then 😋

if we can get synonym aliases for defs for backwards compatibility for enter, p, n, g.

do you mean adding Nushell aliases in default_config.nu? 🤔

this probably also needs a git merge main since it's a bit old now.

there is no conflict, you just want to re run the CI with the latest main for integration?

do you mean adding Nushell aliases in default_config.nu? 🤔

I meant in the module itself so that anyone who uses it will have the backwards compatible usage.

there is no conflict, you just want to re run the CI with the latest main for integration?

Even though the CI is green, there have been changes since the PR as submitted and sometimes we get into trouble if we have PRs that have set for a long time and then land them.

I think the dirs module could export the n, p etc. as aliases to the dirs commands and these could be added to the prelude to be loaded automatically. This way we don't rely on users periodically checking and updating their config after every release.

I think the dirs module could export the n, p etc. as aliases to the dirs commands and these could be added to the prelude to be loaded automatically. This way we don't rely on users periodically checking and updating their config after every release.

This is what I meant, just said better. 😄

first, let's run the CI... ♻️

this should be better with the last two commits 😌

@amtoine I think this is close.

g isn't dirs show. g goes to a particular index. It looks like maybe we need a dirs goto something like:

alias g = dirs goto
def 'dirs goto' [idx: int] {
  # bounds checking is done in _fetch
  _fetch $idx
}

or maybe just

alias g = _fetch

I'm not sure either of these work but I think you get my point.

We have a dirs drop which works like exit with the shells commands but I don't think we should alias exit to something else. So, this is kind of a missing link. maybe alias dexit = dirs drop?

very good points @fdncred

• i think g should be closer to the old built-in one now with 662056e
• and new dexit alias in f21a64b

ok, let's try this removal and see if anyone notices.

because we added the compatibility aliases, i'm wondering if this still is a breaking change 🤔

because we added the compatibility aliases, i'm wondering if this still is a breaking change 🤔

i think so, if only because exit is not dexit for the new shells commands. or maybe mostly-non-breaking, and only 1 command is a breaking change. not sure of a good way to state that.

if there is a single tiny detail which is a breaking change, then the whole thing is a breaking change 😌