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

improve completions of use and overlay use #11330

Merged
merged 8 commits into from Dec 19, 2023

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Dec 14, 2023

Description

this PR is two-fold

  • make use and overlay use use the same completion algorithm in 48f29b6
  • list directory modules in completions of both with 402acde

User-Facing Changes

i currently have the following in my NU_LIB_DIRS

click to see the script
for dir in $env.NU_LIB_DIRS {
    print $dir
    print (ls $dir --short-names | select name type)
}
/home/amtoine/.local/share/nupm/modules
#┬────────name────────┬type
0│nu-git-manager      │dir
1│nu-git-manager-sugar│dir
2│nu-hooks            │dir
3│nu-scripts          │dir
4│nu-themes           │dir
5│nupm                │dir
─┴────────────────────┴────

/home/amtoine/.config/nushell/overlays
#┬──name──┬type
0│ocaml.nu│file
─┴────────┴────

Note
all the samples below are run from the Nushell repo, i.e. a directory with a toolkit.nu module

before the changes

  • use would give me ["ocaml.nu", "toolkit.nu"]
  • overlay use would give me []

after the changes

both commands give me

[
    "nupm/",
    "ocaml.nu",
    "toolkit.nu",
    "nu-scripts/",
    "nu-git-manager/",
    "nu-git-manager-sugar/",
]

Tests + Formatting

  • adds a new directory_completion/mod.nu to the completion fixtures
  • make sure source-env, use and overlay-use are all tested in the dotnu test
  • fix all the other tests that use completions in the fixtures directory for completions

After Submitting

this checks if `search dir + it + mod.nu` exists and adds that to the
completion items if so.
i renamed the outter `it` to `search_dir` to avoid it being shadowed by
the innermost one and avoid defining a binding with the same name.
@amtoine amtoine added the completions Issues related to tab completion label Dec 16, 2023
@amtoine
Copy link
Member Author

amtoine commented Dec 16, 2023

the CI does not pass in the last commit because the new completion directory module broke other tests.
but we can see the CI goes from 13 to 12 failed tests, and the test that passes in between is the dotnu test 👌
i'll fix the other ones right now

@amtoine amtoine marked this pull request as ready for review December 16, 2023 13:05
Copy link
Collaborator

@WindSoilder WindSoilder left a comment

Choose a reason for hiding this comment

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

Thanks! It looks good to me

@WindSoilder WindSoilder merged commit c6043eb into nushell:main Dec 19, 2023
19 checks passed
@amtoine amtoine deleted the better-use-completions branch December 19, 2023 10:07
@amtoine
Copy link
Member Author

amtoine commented Dec 19, 2023

@WindSoilder
the only thing now that feels imperfect is that the dotnu completion does not only completes .nu files, but also directory modules 🤔

@WindSoilder
Copy link
Collaborator

Isn't it expected? It seems that it only shows module if it contains mod.nu

@amtoine
Copy link
Member Author

amtoine commented Dec 19, 2023

yeah, i think it's expected 👍
i meant the name itself, dotnu, is not what's happening right now 😮
it's more like NuModuleCompletion 😉

dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description
this PR is two-fold
- make `use` and `overlay use` use the same completion algorithm in
48f29b6
- list directory modules in completions of both with 402acde

# User-Facing Changes
i currently have the following in my `NU_LIB_DIRS`
<details>
<summary>click to see the script</summary>

```nushell
for dir in $env.NU_LIB_DIRS {
    print $dir
    print (ls $dir --short-names | select name type)
}
```
</details>

```
/home/amtoine/.local/share/nupm/modules
#┬────────name────────┬type
0│nu-git-manager      │dir
1│nu-git-manager-sugar│dir
2│nu-hooks            │dir
3│nu-scripts          │dir
4│nu-themes           │dir
5│nupm                │dir
─┴────────────────────┴────

/home/amtoine/.config/nushell/overlays
#┬──name──┬type
0│ocaml.nu│file
─┴────────┴────
```

> **Note**
> all the samples below are run from the Nushell repo, i.e. a directory
with a `toolkit.nu` module

## before the changes
- `use` would give me `["ocaml.nu", "toolkit.nu"]` 
- `overlay use` would give me `[]` 

## after the changes
both commands give me
```nushell
[
    "nupm/",
    "ocaml.nu",
    "toolkit.nu",
    "nu-scripts/",
    "nu-git-manager/",
    "nu-git-manager-sugar/",
]
```

# Tests + Formatting
- adds a new `directory_completion/mod.nu` to the completion fixtures
- make sure `source-env`, `use` and `overlay-use` are all tested in the
_dotnu_ test
- fix all the other tests that use completions in the fixtures directory
for completions

# After Submitting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completions Issues related to tab completion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants