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

feat(cli)!: actually switch directory with --directory/-C #9831

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

finswimmer
Copy link
Member

@finswimmer finswimmer commented Nov 6, 2024

Pull Request Check List

--directory now switch the current working folder, so that relative paths are resolved relative to the given folder.

To retain the old behavior the --project flag is introduced.

Resolves: #7897

  • Added tests for changed code.
  • Updated documentation for changed code.

@finswimmer finswimmer added impact/docs Contains or requires documentation changes impact/changelog Requires a changelog entry labels Nov 6, 2024
@finswimmer finswimmer force-pushed the directory-change branch 6 times, most recently from 2b8a3b6 to 85d74a7 Compare November 8, 2024 08:40
Copy link

github-actions bot commented Nov 8, 2024

Deploy preview for website ready!

✅ Preview
https://website-fby04ghrm-python-poetry.vercel.app

Built with commit e5e553b.
This pull request is being automatically deployed with vercel-action

@finswimmer finswimmer marked this pull request as ready for review November 8, 2024 08:42
@finswimmer finswimmer requested a review from a team November 8, 2024 08:42
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

Can we add some sort of test for the new option?

@finswimmer
Copy link
Member Author

Can we add some sort of test for the new option?

I really would like to add some tests. But until now I wasn't able to do so because I don't know how.

The scenario I have in mind goes like this:

  • install a Poetry project with a tool.poetry.scripts entrypoint
  • This entrypoint simply prints out the current working directory
  • run poetry run from outside the project folder with either --directory or --project and compare the output of the script with the expected

So any help is appreciated 😃

@radoering
Copy link
Member

Most similar test is probably

def test_run_script_sys_argv0(

which runs a scripts entrypoint and checks the error output.

In https://github.com/python-poetry/poetry/tree/main/tests/fixtures/scripts, there is a project, which already has some entry points. There, you can add another one.

@finswimmer
Copy link
Member Author

Thanks @radoering. 👍 I played around a while, but due to the creation of venv I find it to complicated. In finally choose the version command to demonstrate what should happen when using the parameters.

docs/cli.md Outdated Show resolved Hide resolved
@finswimmer finswimmer force-pushed the directory-change branch 2 times, most recently from 805c746 to 6322aa7 Compare November 17, 2024 17:25
Copy link
Member

@Secrus Secrus left a comment

Choose a reason for hiding this comment

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

What is the expected behaviour when user passes both --project and --directory? I feel like there should be some more info on that matter, it doesn't feel intuitive right now and the docs are quite vague on the difference between those two.

src/poetry/console/application.py Outdated Show resolved Hide resolved
@radoering
Copy link
Member

What is the expected behaviour when user passes both --project and --directory?

I did not check, but my expectation is that the working directory is changed but the pyproject.toml is searched in --project.

I feel like there should be some more info on that matter, it doesn't feel intuitive right now and the docs are quite vague on the difference between those two.

I agree it makes sense to add a sentence about this special case. Maybe, we should even add a test case where both parameters are used.

@finswimmer
Copy link
Member Author

I agree it makes sense to add a sentence about this special case. Maybe, we should even add a test case where both parameters are used.

I've added a test case. Any suggestion for extending the docs?

@finswimmer finswimmer requested review from Secrus and abn November 18, 2024 12:48
@radoering
Copy link
Member

Any suggestion for extending the docs?

Maybe an info box below the global options. However, while thinking about what to write I did not come up with something that is saying more than what has already been added in #9831 (comment). Maybe, that is sufficient.

@finswimmer finswimmer merged commit f3deb4c into python-poetry:main Nov 26, 2024
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/changelog Requires a changelog entry impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Actually switch directory with --directory/-C
4 participants