fix(application): handle global options correctly #10021

abn · 2025-01-11T23:08:30Z

Previously, the application implementation did not handle global options correctly when positioned after command options. This lead to incorrect cached working directory values prior to switching application context.

Resolves: #9978 python-poetry/poetry-plugin-shell#5

Summary by Sourcery

Bug Fixes:

Fixed an issue where global options were not handled correctly when positioned after command options, leading to incorrect cached working directory values.

sourcery-ai · 2025-01-11T23:08:33Z

Reviewer's Guide by Sourcery

This pull request fixes a bug where global options were not handled correctly when positioned after command options, leading to incorrect cached working directory values. The fix involves refactoring the Application class to correctly handle global options such as --directory and --project. Several tests were added to cover the different scenarios and ensure the fix is effective.

Sequence diagram for global options handling in Application

sequenceDiagram
    participant App as Application
    participant IO as IO
    participant Dir as Directory Utils

    App->>App: _run(io)
    App->>App: _configure_custom_application_options(io)
    App->>IO: Get option values (directory, project)
    App->>Dir: ensure_path(directory)
    Note right of Dir: Validate directory path
    Dir-->>App: Return validated working_directory
    App->>Dir: ensure_path(project)
    Note right of Dir: Validate project path
    Dir-->>App: Return validated project_directory

Class diagram showing Application class changes

classDiagram
    class Application {
        -_working_directory: Path
        -_project_directory: Path
        -_disable_plugins: bool
        -_disable_cache: bool
        +_run(io: IO): int
        +_configure_custom_application_options(io: IO): void
        +_option_get_value(io: IO, name: str, default: Any): Any
    }

    class Helpers {
        +ensure_path(path: str|Path, is_directory: bool): Path
    }

    Application ..> Helpers: uses

Flow diagram for global options processing

flowchart TD
    A[Start] --> B[Get IO input]
    B --> C{Has directory option?}
    C -->|Yes| D[Validate directory path]
    C -->|No| E[Use current directory]
    D --> F{Has project option?}
    E --> F
    F -->|Yes| G[Validate project path]
    F -->|No| H[Use working directory]
    G --> I[Set working and project directories]
    H --> I
    I --> J[End]

File-Level Changes

Change	Details	Files
Refactored the `Application` class to correctly handle global options (`--directory`, `--project`).	Removed cached properties for `_project_directory` and `_working_directory`. Added a new method `_configure_custom_application_options` to handle global options. Added a helper method `_option_get_value` to retrieve option values. Used `ensure_path` to validate directory paths. Modified `_run` to call `_configure_custom_application_options` before loading plugins. Corrected the logic for determining the project and working directories based on the global options.	`src/poetry/console/application.py`
Added tests to verify the fix and cover different scenarios.	Added tests for various global option positions. Added tests to ensure correct behavior with valid and invalid project/directory paths. Added tests to verify correct project and working directory resolution with relative and absolute paths.	`tests/console/test_application_global_options.py`
Removed unused imports and code related to the old implementation.	Removed unused imports in `test_version.py`. Removed tests that were no longer relevant. Added `ensure_path` function to `helpers.py`. Added tests for `ensure_path` function in `test_helpers.py`.	`tests/console/commands/test_version.py` `tests/utils/test_helpers.py` `src/poetry/utils/helpers.py`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time. You can also use
this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

abn · 2025-01-11T23:22:39Z

The issue reported in #9978 can be reproduced and the fix in this PR tested using the following snippet.

podman run --rm -i --entrypoint bash python:latest <<EOF
set -x
python -m pip install --disable-pip-version-check --root-user-action ignore -q poetry
pushd /tmp
poetry new foobar
poetry add -C /tmp/foobar httpx
poetry show --only main -C /tmp/foobar || :
python -m pip install --disable-pip-version-check --root-user-action ignore -q --force 'git+https://github.com/python-poetry/poetry.git@refs/pull/10021/head'
poetry show --only main -C /tmp/foobar
EOF

sourcery-ai

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

abn · 2025-01-12T19:08:28Z

@sourcery-ai review

sourcery-ai

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

src/poetry/utils/helpers.py

src/poetry/console/application.py

Previously, the application implementation did not handle global options correctly when positioned after command options. This lead to incorrect cached working directory values prior to switching application context. Resolves: python-poetry#9978

abn · 2025-01-12T20:06:30Z

Changed the path resolution for --project to avoid doing a hard cwd switch.

This change ensures that context options like --project and --directory are validated prior to executing the application to prevent runtime exceptions.

This change fixes an issue introduced in python-poetry#10021 that led to options provided in the form `-<shortcut><value>` to be incorrectly handled. Further, it also simplifies and fixes run command processing. Resolves: python-poetry#10051

github-actions · 2025-02-14T00:29:14Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

abn marked this pull request as ready for review January 11, 2025 23:22

abn requested a review from a team January 11, 2025 23:22

sourcery-ai bot reviewed Jan 11, 2025

View reviewed changes

abn mentioned this pull request Jan 11, 2025

poetry 2 install command can not find pyproject.toml with --only and --directory parameters #9978

Closed

abn force-pushed the issue/9978 branch 2 times, most recently from af98156 to 49a09b2 Compare January 12, 2025 19:08

sourcery-ai bot reviewed Jan 12, 2025

View reviewed changes

src/poetry/utils/helpers.py Show resolved Hide resolved

src/poetry/console/application.py Outdated Show resolved Hide resolved

abn force-pushed the issue/9978 branch 2 times, most recently from 89b3d62 to 2cc721c Compare January 12, 2025 19:20

Secrus reviewed Jan 12, 2025

View reviewed changes

src/poetry/console/application.py Outdated Show resolved Hide resolved

src/poetry/console/application.py Show resolved Hide resolved

abn force-pushed the issue/9978 branch from 2cc721c to ac5427c Compare January 12, 2025 20:01

abn added 2 commits January 12, 2025 21:04

chore(test): refactor app context tests

f37ba94

abn force-pushed the issue/9978 branch from ac5427c to 5d3a58a Compare January 12, 2025 20:04

abn requested a review from Secrus January 12, 2025 20:06

abn force-pushed the issue/9978 branch from 5d3a58a to 00cd89d Compare January 12, 2025 20:40

fix(application): check context options preflight

54c3731

This change ensures that context options like --project and --directory are validated prior to executing the application to prevent runtime exceptions.

abn force-pushed the issue/9978 branch from 00cd89d to 54c3731 Compare January 12, 2025 23:38

Secrus approved these changes Jan 13, 2025

View reviewed changes

abn merged commit ff3f8bd into python-poetry:main Jan 13, 2025
73 checks passed

abn deleted the issue/9978 branch January 13, 2025 12:25

abn mentioned this pull request Jan 13, 2025

--with and --directory args are heavily ordering dependent #10036

Closed

abn mentioned this pull request Jan 20, 2025

fix(app): improve global opt and run cmd parsing #10067

Merged

finswimmer mentioned this pull request Feb 6, 2025

release: bump version to 2.1.0 #10154

Merged

4 tasks

finswimmer mentioned this pull request Feb 10, 2025

--project/--directory only work as first cli option #10165

Closed

github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(application): handle global options correctly #10021

fix(application): handle global options correctly #10021

Uh oh!

abn commented Jan 11, 2025 •

edited

Loading

Uh oh!

sourcery-ai bot commented Jan 11, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

abn commented Jan 11, 2025

Uh oh!

sourcery-ai bot left a comment

Uh oh!

abn commented Jan 12, 2025

Uh oh!

sourcery-ai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abn commented Jan 12, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 14, 2025

Uh oh!

Uh oh!

fix(application): handle global options correctly #10021

fix(application): handle global options correctly #10021

Uh oh!

Conversation

abn commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

Sequence diagram for global options handling in Application

Class diagram showing Application class changes

Flow diagram for global options processing

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

abn commented Jan 11, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

abn commented Jan 12, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abn commented Jan 12, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 14, 2025

Uh oh!

Uh oh!

abn commented Jan 11, 2025 •

edited

Loading

sourcery-ai bot commented Jan 11, 2025 •

edited

Loading