Skip to content

Reworking help aliases#8372

Merged
kubouch merged 10 commits intonushell:mainfrom
klementievdmitry:main
Mar 10, 2023
Merged

Reworking help aliases#8372
kubouch merged 10 commits intonushell:mainfrom
klementievdmitry:main

Conversation

@klementievdmitry
Copy link
Copy Markdown
Contributor

@klementievdmitry klementievdmitry commented Mar 9, 2023 •
edited
Loading

Description

I reworked the help aliases command a bit. It now collects old and new aliases

User-Facing Changes

  • previously aliases, were not listed in the help aliases:
$ help aliases | where name == "bar"
╭────────────╮
│ empty list │
╰────────────╯
$ alias bar = echo "this is bar"
$ help aliases | where name == "bar"
╭────────────╮
│ empty list │
╰────────────╯
  • now the aliases, will be listed in help aliases and $nu.scope.aliases:
$ help aliases | where name == "bar"
╭────────────╮
│ empty list │
╰────────────╯
$ $nu.scope.aliases | where name == "bar"
╭────────────╮
│ empty list │
╰────────────╯
$ alias bar = echo "this is bar"
$ help aliases | where name == "bar"
╭───┬──────┬────────────────────┬───────╮
│ # │ name │     expansion      │ usage │
├───┼──────┼────────────────────┼───────┤
│ 0 │ bar  │ echo "this is bar" │       │
╰───┴──────┴────────────────────┴───────╯
$ $nu.scope.aliases | where name == "bar"
╭───┬──────┬────────────────────┬───────╮
│ # │ name │     expansion      │ usage │
├───┼──────┼────────────────────┼───────┤
│ 0 │ bar  │ echo "this is bar" │       │
╰───┴──────┴────────────────────┴───────╯

Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace to check that all tests pass

After Submitting

If your PR had any user-facing changes, update the documentation after the PR is merged, if necessary. This will help us keep the docs up to date.

@klementievdmitry klementievdmitry changed the title Reworking help aliases (#8301) Reworking help aliases Mar 9, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 9, 2023 •
edited
Loading

Codecov Report

Merging #8372 (9533ded) into main (878e08c) will decrease coverage by 0.02%.
The diff coverage is 17.24%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8372      +/-   ##
==========================================
- Coverage   68.49%   68.47%   -0.02%     
==========================================
  Files         620      620              
  Lines       99495    99524      +29     
==========================================
+ Hits        68145    68150       +5     
- Misses      31350    31374      +24
Impacted Files Coverage Δ
crates/nu-engine/src/scope.rs 87.32% <17.24%> (-3.83%) ⬇️

... and 3 files with indirect coverage changes

rgwood
rgwood requested changes Mar 10, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@rgwood rgwood left a comment

Choose a reason for hiding this comment

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

Nice, thanks for fixing this up! Looks good overall, just 2 comments.

@kubouch, are you OK with this or will it conflict with the changes you're making to aliases?

@klementievdmitry
Copy link
Copy Markdown
Contributor Author

klementievdmitry commented Mar 10, 2023 •
edited
Loading

@rgwood, what do you think about moving the implementation from build_help_aliases to collect_aliases? This would also affect $nu.scope.aliases.

@klementievdmitry klementievdmitry marked this pull request as ready for review March 10, 2023 06:36
@klementievdmitry klementievdmitry requested a review from rgwood March 10, 2023 06:37
@rgwood
Copy link
Copy Markdown
Contributor

rgwood commented Mar 10, 2023

I think that's a good idea. Let's wait until @kubouch has time to take a quick look, he's been doing a lot of work in this area recently.

@kubouch
Copy link
Copy Markdown
Contributor

kubouch commented Mar 10, 2023

Cool, looks good, thanks!

@kubouch kubouch merged commit 0ff1cb1 into nushell:main Mar 10, 2023
@kubouch kubouch mentioned this pull request Mar 10, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgwood rgwood Awaiting requested review from rgwood

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@klementievdmitry @rgwood @kubouch