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

Derive settings_path from --filename #1992

Merged
merged 2 commits into from Feb 4, 2023
Merged

Conversation

kaste
Copy link
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).
@@ -1087,7 +1087,9 @@ def main(argv: Optional[Sequence[str]] = None, stdin: Optional[TextIOWrapper] =
return
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()

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
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

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

@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
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

@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