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

Add entrypoints to tool list #4661

Merged
merged 7 commits into from
Jul 3, 2024

Conversation

moreaupascal56
Copy link
Contributor

@moreaupascal56 moreaupascal56 commented Jun 30, 2024

Closes #4654

Summary

The purpose of this is to show the entrypoints of each tool when running uv tool list as below:

$ uv tool list
black
    black
    blackd

I used the proposed formatting as it was written in #4653 by @blueraft.
I had to use spaces instead of tabs in order to make the test successful. Indeed in the test we are using a raw string and I did not manage to make the test pass when escaping the tab in the list.rs file so I used spaces everywhere.

I had a deeper look into #4653 as well but it is more difficult as we need to get the version of the tool in the Tool object, I will continue on this next one later.

Please tell me if anything else is needed I tried to follow the contribution guidelines but I might have forgotten something.
Have a great day!

Test Plan

cargo clippy

then by using the local version of uv as described in the Readme.md.

my-computer :~/mypath/uv$ cargo run -- tool list
   Compiling uv-cli v0.0.1 (/mypath/uv/crates/uv-cli)
   Compiling uv v0.2.18 (/mypath/uv/crates/uv)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 18.69s
     Running `target/debug/uv tool list`
warning: `uv tool list` is experimental and may change without warning.
black
  black
  blackd
isort
  isort
  isort-identify-imports

and

cargo test tool_list

@moreaupascal56 moreaupascal56 changed the title Add entrypoints to tool list Draft: Add entrypoints to tool list Jun 30, 2024
@moreaupascal56 moreaupascal56 changed the title Draft: Add entrypoints to tool list Add entrypoints to tool list Jun 30, 2024
@zanieb zanieb self-assigned this Jul 1, 2024
@zanieb
Copy link
Member

zanieb commented Jul 1, 2024

Thank you for contributing!

@moreaupascal56
Copy link
Contributor Author

Hi, so I fixed the issue but the windows test is failing because of something else. Here is the test output:

        FAIL [   2.335s] uv::tool_list tool_list

--- STDOUT:              uv::tool_list tool_list ---

running 1 test
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: tool_list
Source: E:\uv:24
───────────────────────────────────────────────────────────────────────────────
Expression: snapshot
───────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬──────────────────────────────────────────────────────────────────
    0     0 │ success: true
    1     1 │ exit_code: 0
    2     2 │ ----- stdout -----
    3     3 │ black
    4       │-    black
    5       │-    blackd
          4 │+    black.exe
          5 │+    blackd.exe
    6     6 │ 
    7     7 │ ----- stderr -----
    8     8 │ warning: `uv tool list` is experimental and may change without warning.
────────────┴──────────────────────────────────────────────────────────────────
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
test tool_list ... FAILED

failures:

failures:
    tool_list

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 2.32s


--- STDERR:              uv::tool_list tool_list ---


Indeed in windows the entrypoint name now ends with .exe.
This test worked yesterday and i do not think the former loop was removing the ".exe" ends of the Strings (I might miss something obvious 🤔 ).
I guess this has been introduced from this PR #4693 but I am unsure about this assumption.

Should we keep this .exe in the output (then adapting the tests accordingly) OR remove the .exe from the string (ie. either an if clause or somthing like entrypoint.replace(".exe","") to have a unified output in all OS?

@zanieb
Copy link
Member

zanieb commented Jul 3, 2024

Yep you can use let context = TestContext::new("3.12").with_filtered_exe_suffix(); to turn on that filtering automatically :)

@zanieb zanieb enabled auto-merge (squash) July 3, 2024 04:33
@zanieb zanieb disabled auto-merge July 3, 2024 04:33
@zanieb
Copy link
Member

zanieb commented Jul 3, 2024

Nice work! Thanks again :)

@zanieb zanieb merged commit 174414c into astral-sh:main Jul 3, 2024
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show tool entry points in uv tool list
2 participants