inquire icon indicating copy to clipboard operation
inquire copied to clipboard

Filepath select

Open anwarhahjjeffersongeorge opened this issue 1 year ago • 9 comments

Hi. I tried to make a file path selector that:

  • Is its own prompt (PathSelect) with optional configuration (PathSelectPrompt) gated behind the path feature
  • Is OS-agnostic,
  • Uses the fs_err crate where possible for more detailed diagnostics,
  • Allows starting from a custom file path start_path_opt (defaulting to current process directory or root path if current process directory is unavailable)
  • Allows selection of one or multiple files with configuration option select_multiple,
  • Allows selecting (with select_multiple enabled) files from more than one directory,
  • Allows filtering of selectable file types by directory, extension, or a combination of multiple filters selection_mode,
  • Allows following/showing symbolic links with configuration option show_symlinks.
  • Allows showing hidden files with configuration option show_hidden,
  • Uses Rust's std::path and std::ffi::OsStr for portability instead of trying to figure out whether to convert paths to strings, and
  • Includes an example (path_select.rs).

Deficiencies

It does not:

  • Deal with permissions (it's possible, but I didn't know if it was a good idea),
  • Use existing prompts, although it is based heavily on the MultiSelect prompt code,
  • Handle formatting very aesthetically,
  • Support certain types of hidden files as I neither have a windows machine for testing nor understand how it works :(,
  • Support user-provided custom option filtering beyond selecting acceptable file types using the PathSelectionMode type ,
  • Support user-provided validation functions,
  • Show relative paths (there are crates to do this, but they seem to have some open issues), or
  • Have very many tests. I didn't know how you wanted tests written. For what it's worth, everything passes when I run cargo test --all-features-. It seems to work when I tried it on Ubuntu 23.04 and Mac OS 10.15.7.

I am opening this PR mainly for feedback. I know the contributor's guide said to do the discussion stage first, but I needed this feature for a project and did not see that detail until after I made it. I saw some existing issues (#124, #63), and tried to go from there, favoring the strategy of 63 ( a new prompt type) with the features of 124 (as detailed above).

With all that in mind, I understand if this needs more work.

I really like this. And thank you for the attention to detail on the PR description lol.

Hopefully I'll find some time tomorrow for a full review.

One important point that I'd bring up is that recently (like yesterday maybe) I've refactored prompts. Far from a revamp but significant enough that you might want to get ahead in porting this PR already. Honestly, I can also do that myself if you'd prefer, whatever works.

mikaelmello avatar May 09 '23 06:05 mikaelmello

I gave refactoring the prompts a shot. For now, I'll chill out on this until whenever you get a chance to look and let me know what needs improvement.

I do feel bad about dumbing a lot of comments, but it is a very large feature :). I want it to be as good as possible before releasing, which today is basically the same as merging.

I was thinking, we should break this down in smaller chunks and get the thing going more gradually, we can merge but not expose this on the published crate until we gradually get this to the end state.

I think this would also allow us to work in parallel.

Also, obligatory sorry for taking too long on this round of reviews

mikaelmello avatar Jun 11 '23 02:06 mikaelmello

Hi,

I am also interested in this.

Did this go anywhere? Any way I can help?

Barre avatar Aug 01 '23 10:08 Barre

@mikaelmello @Barre Hey there, I missed this before, but I'll take a look at these next weekend.

I was thinking, we should break this down in smaller chunks and get the thing going more gradually, we can merge but not expose this on the published crate until we gradually get this to the end state.

I think this would also allow us to work in parallel.

Just let me know how you'd like me to reorganize it that would work for you.

Also, obligatory sorry for taking too long on this round of reviews

No problem. It's been like 110+ degrees where I live, so haven't exactly been rushing to turn the computer on and heat up the house.

any progress with this?

DaRacci avatar Feb 08 '24 05:02 DaRacci

any progress with this? @DaRacci @mikaelmello Said above they wanted me to reorganize or break this down but I'm not sure what that meant, so I'm waiting on that before doing more with this.

hey, I know I've been really dropping the ball on being responsive throughout inquire, life getting in the way as usual

I really really want to see this merged, the main point holding it back is that this is a very significant prompt to offer, counter-intuitively.

it's one of those things I'd like to polish and re-polish it before releasing, making sure the API is very intuitive and everything

mikaelmello avatar Mar 13 '24 05:03 mikaelmello