Skip to content

Derive settings_path from --filename#1992

Merged
timothycrosley merged 2 commits intoPyCQA:mainfrom
kaste:fix-1989
Feb 4, 2023
Merged

Derive settings_path from --filename#1992
timothycrosley merged 2 commits intoPyCQA:mainfrom
kaste:fix-1989

Conversation

@kaste
Copy link
Copy Markdown
Contributor

@kaste kaste commented Nov 11, 2022

Fixes #1989

--filename is a shortcut setting used only in the stdin case to set
the settings_path.
Generally, --settings-path=<file_path> == --filename=<filename>
should hold.

This functionality was removed in 0973421 (Reuse config when items
passed in through stdin as used when items passed in explicitly).

Remove the or-clause because `os.path.abspath(".")` equals
`os.getcwd()`.
Fixes PyCQA#1989

`--filename` is a shortcut setting used only in the stdin case to set
the `settings_path`.
Generally, `--settings-path=<file_path>` == `--filename=<filename>`
should hold.

This functionality was removed in 0973421 (Reuse config when items
passed in through stdin as used when items passed in explicitly).
Comment thread isort/main.py
if "settings_path" not in arguments:
arguments["settings_path"] = (
os.path.abspath(file_names[0] if file_names else ".") or os.getcwd()
arguments.get("filename", None) or os.getcwd()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
arguments.get("filename", None) or os.getcwd()
os.path.abspath(arguments.get("filename", None) or os.getcwd())

Should --filename argument be wrapped with abspath as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that's confusing because then it depends on the working dir again. The working dir is not as easy to get right for editors; it might be the topmost open folder, or the first folder with .git in it. And then we need to construct a relative filename of course to set it as the argument.

@LukasVik
Copy link
Copy Markdown

I have checked out this branch and tried it in VSCode, and this does indeed solve all issues I had with #1989 and microsoft/vscode-isort#53.

Would it be possible to merge this soon?

I unfortunately have no input regarding the abspath or not.

@karthiknadig
Copy link
Copy Markdown

@kaste This bug affects our (VS Code extension for isort) ability to provide import sorting on unsaved files. If this can be merged soon it would be greatly helpful to improve the experience.

@kaste
Copy link
Copy Markdown
Contributor Author

kaste commented Feb 2, 2023

@karthiknadig I just did the PR here to fix the bug. I have no merge rights whatsoever. 🤷

@karthiknadig
Copy link
Copy Markdown

@timothycrosley would it be possible for you to take a look at this?

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.

isort does not search for config when using --filename

4 participants