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

Show file picker when opening a directory #59

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

vv9k
Copy link
Contributor

@vv9k vv9k commented Jun 2, 2021

No description provided.

@vv9k
Copy link
Contributor Author

vv9k commented Jun 2, 2021

So this is what i initially came up with. It will open all files as splits, the last path that is a directory will end up as file pickers root. All other directories will be silently ignored.

Here is an example of invoking:

hx /tmp Cargo.toml Cargo.lock

Upon user selection from file picker, the file will end up in the split that is empty as it is focused.

screenshot of hx

@pickfire
Copy link
Contributor

pickfire commented Jun 2, 2021

I don't think this is a good idea. It's quite weird to take in both directory and files, and what if users send in two directories? Show two file picker? I think it is hard to estimate the behavior, a better option would be to give an error saying users cannot send in directory and file together.

@vv9k vv9k marked this pull request as ready for review June 2, 2021 15:52
@vv9k
Copy link
Contributor Author

vv9k commented Jun 2, 2021

So basically only allow either a single directory or single/multiple files?

@pickfire
Copy link
Contributor

pickfire commented Jun 2, 2021

That's what I think, not sure if others have this in mind as well.

@vv9k vv9k changed the title Show file picker when at least one directory is passed Show file picker when opening a directory Jun 2, 2021
@vv9k
Copy link
Contributor Author

vv9k commented Jun 2, 2021

I do like this behaviour better so I updated the code to reflect that.

@IceDragon200
Copy link
Contributor

I think iterating over the files at least once to detect their type first would be ideal, instead of pushing contexts only to error later once another directory is found.

Some pseudo code

let file_count = 0;
let dir_count = 0;

for file in files {
  if (file.is_dir()) {
    dir_count += 1;
  } else {
    file_count += 1;
  }
}

if (dir_count > 1) {
  // raise here, we can't handle more than one directory
  // we also don't waste clock cycles on opening buffers that will be discarded because of the panic
}

if (file_count > 0) {
  // open the buffers
}

if (dir_count > 0) {
  // open the file picker also
}

if (file_count == 0 and dir_count == 0) {
  // open default buffer instead
}

You could also replace the counters with a vector instead to collect the files that should be opened in buffers before finally opening the file picker for the directory if needed

Just my 2-cents

@pickfire
Copy link
Contributor

pickfire commented Jun 3, 2021

@IceDragon200 I don't agree. We can just stop once we hit a weird case. So if the first argument is file the rest is file, once a directory is found error. But directory should only be able to open for one directory, so if there are more arguments for directory, error.

@archseer
Copy link
Member

archseer commented Jun 4, 2021

Thanks! 🎉

@archseer archseer merged commit 43b92b2 into helix-editor:master Jun 4, 2021
@vv9k vv9k deleted the picker branch June 9, 2021 05:45
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.

None yet

4 participants