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.

Sign up for GitHub

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

Jump to bottom

Resolve args.files before changing directory #8676

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Resolve args.files before changing directory #8676

merged 5 commits into from
Nov 21, 2023

Conversation

bjorn-ove
Copy link
Contributor

As discussed with @the-mikedavis in #8520 (comment)

@pascalkuthe
Copy link
Member

pascalkuthe commented Oct 31, 2023 •
edited
Loading

I think you can now simplify the logic to open the CWD again (get rid of the open_cwd flags and instead simply restore the handling of the first argument to how it was before #8520)

@bjorn-ove
Copy link
Contributor Author

Good point

@bjorn-ove
Copy link
Contributor Author

One question that may be worth asking now is what is the logical behaviour of running helix -w ~/.config ~/.ssh or helix -w ~/.config ~/.config/helix

Currently it will effectively ignore -w, but perhaps it is better to change this, so it honors -w for working directory, and opens a picker at the first argument?

@the-mikedavis
Copy link
Member

Yeah, I would prefer -w to take precedence, so we would set the cwd to ~/.config in that first example and open a picker for ~/.ssh

@bjorn-ove
Copy link
Contributor Author

Ok, I made two changes now.

The first as discussed above, the second to open the remaining files in args.files when the first one is a directory.

@bjorn-ove bjorn-ove force-pushed the resolve-paths-before-chdir branch from 11e28df to 7ec1d88 Compare November 7, 2023 08:28
@bjorn-ove bjorn-ove force-pushed the resolve-paths-before-chdir branch from 7ec1d88 to d5fabdd Compare November 7, 2023 08:36
@bjorn-ove
Copy link
Contributor Author

Updated after feedback

@pascalkuthe pascalkuthe merged commit 47b6c4b into helix-editor:master Nov 21, 2023
6 checks passed
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
@bjorn-ove @dgkf
Resolve args.files before changing directory (helix-editor#8676)
65209d5 
* Resolve args.files before changing directory

* Removed the open_cwd work-around now that the path is full

* If -w is specified, use that as the working directory

* Open the remaining files in the argument list, also when the first is a directory

* Use an iterator access the files argument
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
@bjorn-ove @mtoohey31
Resolve args.files before changing directory (helix-editor#8676)
02d8583 
* Resolve args.files before changing directory

* Removed the open_cwd work-around now that the path is full

* If -w is specified, use that as the working directory

* Open the remaining files in the argument list, also when the first is a directory

* Use an iterator access the files argument
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
@bjorn-ove @Vulpesx
Resolve args.files before changing directory (helix-editor#8676)
b9122f1 
* Resolve args.files before changing directory

* Removed the open_cwd work-around now that the path is full

* If -w is specified, use that as the working directory

* Open the remaining files in the argument list, also when the first is a directory

* Use an iterator access the files argument
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
@bjorn-ove
Resolve args.files before changing directory (helix-editor#8676)
51fd6fc 
* Resolve args.files before changing directory

* Removed the open_cwd work-around now that the path is full

* If -w is specified, use that as the working directory

* Open the remaining files in the argument list, also when the first is a directory

* Use an iterator access the files argument
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalkuthe pascalkuthe pascalkuthe approved these changes

@the-mikedavis the-mikedavis the-mikedavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bjorn-ove @pascalkuthe @the-mikedavis