Skip to content

FEATURE: fix the namespace of the standard library (not testing)#9193

Merged
kubouch merged 16 commits intonushell:mainfrom
amtoine:feature/stdlib/fix-the-namespace
May 19, 2023
Merged

FEATURE: fix the namespace of the standard library (not testing)#9193
kubouch merged 16 commits intonushell:mainfrom
amtoine:feature/stdlib/fix-the-namespace

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented May 14, 2023

should close the "structuring" task of

Description

up until now, we could not do the following

> use std log
Error: nu::parser::export_not_found

  × Export not found.
   ╭─[entry #1:1:1]
 1 │ use std log
   ·         ─┬─
   ·          ╰── could not find imports
   ╰────

or

> use std log info
Error: nu::parser::export_not_found

  × Export not found.
   ╭─[entry #2:1:1]
 1 │ use std log info
   ·         ─┬─
   ·          ╰── could not find imports
   ╰────

this PR fixes the namespace by allowing these kind of imports!

User-Facing Changes

the users are now more free to import std items

✔️ valid imports

  • use std log (exposes all log commands)
  • use std log custom (exposes log custom as custom)
  • use std "log info" (as before) (exposes log info as log info)

Warning
to avoid built-in shadowing, this PR changes some command names

  • assert length is now assert len
  • assert str contains is now `assert str includes

Tests + Formatting

  • 🟢 toolkit fmt
  • 🟢 toolkit clippy
  • toolkit test
  • toolkit test stdlib

After Submitting

$nothing

amtoine added 11 commits May 14, 2023 09:44
This commit changed the aliases to `dirs ...` and thus requires
to load (`foo`, `dirs foo`) in the prelude.
This commit
- refactors the `assert` commands into an exported module
- avoids built-in shadowing by renaming
    - `assert length` into `assert len`
    - `assert str contains` into `assert str includes`
@amtoine amtoine added the A:std-library Defining and improving the standard library written in Nu label May 14, 2023
@amtoine amtoine self-assigned this May 14, 2023
@amtoine amtoine added the notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes label May 14, 2023
@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 14, 2023

i would like to make std testing assert called as std assert as before 🤔
working on it... ♻️

EDIT: locally in my tests

  • use std run-tests works as before
  • use std assert only imports the assert command
  • use std assert equal only imports assert
  • use std "assert equal" import assert equal

@amtoine amtoine marked this pull request as draft May 14, 2023 08:49
@amtoine amtoine mentioned this pull request May 14, 2023
7 tasks
@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 18, 2023

there is definitely an issue with main commands inside the modules of std 🤔
i could not find a solution so i reverted the changes made to the testting module...
at least all the rest is fixed 😌

@amtoine amtoine marked this pull request as ready for review May 18, 2023 12:24
@amtoine amtoine changed the title FEATURE: fix the namespace of the standard library FEATURE: fix the namespace of the standard library (not testing) May 18, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented May 18, 2023

Codecov Report

Merging #9193 (a446a38) into main (bf86cd5) will decrease coverage by 0.40%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9193      +/-   ##
==========================================
- Coverage   68.92%   68.53%   -0.40%     
==========================================
  Files         635      636       +1     
  Lines      102041   102281     +240     
==========================================
- Hits        70332    70097     -235     
- Misses      31709    32184     +475     
Impacted Files Coverage Δ
crates/nu-std/src/lib.rs 81.98% <100.00%> (ø)

... and 24 files with indirect coverage changes

@kubouch
Copy link
Copy Markdown
Contributor

kubouch commented May 19, 2023

I'll merge this because I need to iterate on this in #9245. Seems good to me, it's better not to have the prefixes on the command names.

@amtoine Can you describe the main issue you ran into?

@kubouch kubouch merged commit 55bb501 into nushell:main May 19, 2023
@amtoine amtoine deleted the feature/stdlib/fix-the-namespace branch May 20, 2023 11:02
@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 20, 2023

@kubouch

Can you describe the main issue you ran into?

yes i can 💪

Note
the command below are all run from inside cargo run -- -n while being checked out on the revision

✔️ on latest main, i.e. 429c433, if you use std iter, you'll have access to all the iter commands.
you can try hitting tab after iter and see iter find, iter scan, etc, etc, ...

now let's say we want to add a main command to the module to give the help of the module itself

export def main [] { help iter }

at the end of iter.nu
now we get the following behaviours

  • use std iter only import the iter command, which does print some incomplete help
  • use std iter find does not import iter find but only iter, no find to be seen
  • ✔️ use std and then std iter works
  • ✔️ use std "iter find" does work as expected

amtoine added a commit to amtoine/nushell that referenced this pull request May 20, 2023
amtoine added a commit that referenced this pull request May 20, 2023
related to the changes in
- #9193

# Description
when we change the namespace of a module, the internal calls to the
`export`ed commands needs to be updated as well 👀 😆

without this, we have the following pretty error:
```
> std help ansi
Error:   × std::help::item_not_found
   ╭─[entry #1:1:1]
 1 │ std help ansi
   ·          ──┬─
   ·            ╰── item not found
   ╰────
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:std-library Defining and improving the standard library written in Nu notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants