-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
File picker config #988
File picker config #988
Conversation
May be relevant to #983. |
#648 May also be relevant. |
5d6d5ce
to
57a19b2
Compare
8f8e9de
to
e049182
Compare
helix-view/src/editor.rs
Outdated
@@ -61,6 +61,7 @@ pub struct Config { | |||
pub completion_trigger_len: u8, | |||
/// Whether to display infoboxes. Defaults to true. | |||
pub auto_info: bool, | |||
pub hide_gitignore: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's match the name on walk builder: git_ignore
. This also needs to be documented in the book/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have addressed these changes on commit 45c131c.
@@ -93,13 +93,14 @@ pub fn regex_prompt( | |||
) | |||
} | |||
|
|||
pub fn file_picker(root: PathBuf) -> FilePicker<PathBuf> { | |||
pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePicker<PathBuf> { | |||
use ignore::{types::TypesBuilder, WalkBuilder}; | |||
use std::time; | |||
|
|||
// We want to exclude files that the editor can't handle yet | |||
let mut type_builder = TypesBuilder::new(); | |||
let mut walk_builder = WalkBuilder::new(&root); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's another WalkBuilder
in commands.rs used for global search, we should pass this option there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thank you for review, I will look into this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit 339325f I added a variable to allow for passing into .git_ignore()
argument without fiddling with explicit lifetimes, and then inserted this call right after WalkBuilder::new(...)
is called.
The remaining changes were all due to formatting, using Helix with rust-analyser.
helix-view/src/editor.rs
Outdated
@@ -92,6 +94,7 @@ impl Default for Config { | |||
idle_timeout: Duration::from_millis(400), | |||
completion_trigger_len: 2, | |||
auto_info: true, | |||
git_ignore: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where @pickfire's comment went but this should indeed be true to match the current default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thank you for your note.
I am seeing with this, and also the next comment as saying you'd like to define defaults based on the ignore
crate, specifically leaving in the default filters.
helix-view/src/editor.rs
Outdated
Self { | ||
// could simply use .standard_filters if these are uniform. | ||
// Enables ignoring hidden files. | ||
hidden: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The defaults should match the WalkBuilder
defaults.
hidden: true, ignore: true, git-ignore: true, git-global: true, git-exclude: true, follow-links: false, max-depth: None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will set defaults to match the ignore
crate's in my forthcoming commits.
book/src/configuration.md
Outdated
@@ -23,6 +23,15 @@ To override global configuration parameters, create a `config.toml` file located | |||
| `idle-timeout` | Time in milliseconds since last keypress before idle timers trigger. Used for autocompletion, set to 0 for instant. | `400` | | |||
| `completion-trigger-len` | The min-length of word under cursor to trigger autocompletion | `2` | | |||
| `auto-info` | Whether to display infoboxes | `true` | | |||
| `file-picker` | Sets options for file picker and global search. Multiple pairs in an inline table. Details below. | `{hidden = true, parents = true, ignore = false, git-ignore = true, git-global = true, git-exclude = true } | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to move this to an "[editor.file-picker]
section" like the "[editor]
section" above and have the same key, description, default table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I will make a commit for this today. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the commit 80ba0e0 has the changes you were looking for!
helix-view/src/editor.rs
Outdated
// IgnoreOptions | ||
// Enables ignoring hidden files. | ||
hidden: true, | ||
// Enables reading ignore files from parent directories. | ||
parents: true, | ||
// Enables reading `.ignore` files. | ||
ignore: true, | ||
// Enables reading `.gitignore` files. | ||
/// Whether to hide files in .gitignore from displaying in file picker. Defaults to false. | ||
git_ignore: true, | ||
// Enables reading global .gitignore, whose path is specified in git's config: `core.excludefile` option. | ||
git_global: true, | ||
// Enables reading `.git/info/exclude` files. | ||
git_exclude: true, | ||
// WalkBuilder options | ||
// Maximum Depth to recurse. | ||
max_depth: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments should really be doc comments (///
) on the FilePickerConfig
struct, that way they show in crate docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I saw that was the way with previous config, now that makes sense. I'll make an adjustment.
helix-view/src/editor.rs
Outdated
/// Whether to hide files in .gitignore from displaying in file picker. Defaults to false. | ||
//pub git_ignore: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good.
match all_matches_sx_cl | ||
.send((line_num as usize - 1, dent.path().to_path_buf())) | ||
{ | ||
Ok(_) => Ok(true), | ||
Err(_) => Ok(false), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match all_matches_sx_cl | |
.send((line_num as usize - 1, dent.path().to_path_buf())) | |
{ | |
Ok(_) => Ok(true), | |
Err(_) => Ok(false), | |
} | |
Ok(all_matches_sx_cl | |
.send((line_num as usize - 1, dent.path().to_path_buf())) | |
.is_ok()) |
This can probably be simplified, but probably needs reformatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not believe I changed this logic in the course of this PR--the formatting changed due to the addition of the fluent-style calls above (lines 1392-1398).
helix-view/src/editor.rs
Outdated
pub struct FilePickerConfig { | ||
/// IgnoreOptions | ||
/// Enables ignoring hidden files. | ||
/// Whether to hide, that is, to stop hidden files from displaying in file picker. Defaults to false. | ||
pub hidden: bool, | ||
/// Enables reading ignore files from parent directories. | ||
pub parents: bool, | ||
/// Enables reading `.ignore` files. | ||
/// Whether to hide files listed in .ignore from displaying in file picker. Defaults to false. | ||
pub ignore: bool, | ||
/// Enables reading `.gitignore` files. | ||
/// Whether to hide files in .gitignore from displaying in file picker. Defaults to false. | ||
pub git_ignore: bool, | ||
/// Enables reading global .gitignore, whose path is specified in git's config: `core.excludefile` option. | ||
/// Whether to hide files in global .gitignore from displaying in file picker. Defaults to false. | ||
pub git_global: bool, | ||
/// Enables reading `.git/info/exclude` files. | ||
/// Whether to hide files in .git/info/exclude from displaying in file picker. Defaults to false. | ||
pub git_exclude: bool, | ||
/// WalkBuilder options | ||
/// Maximum Depth to recurse. | ||
pub max_depth: Option<usize>, | ||
} | ||
|
||
impl Default for FilePickerConfig { | ||
fn default() -> Self { | ||
Self { | ||
hidden: true, | ||
parents: true, | ||
ignore: true, | ||
git_ignore: true, | ||
git_global: true, | ||
git_exclude: true, | ||
max_depth: None, | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments seemed confusing to me. It's good to put the defaults in comments, but it doesn't seemed to match the defaults it code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some updates to these comments and to the book, hope they are more clear and accurate!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should get rid of mentioning defaults in comments altogether since it could get outdated, and for the book we could do something like #1119.
Does this change allow to set gitignore file picker behaviour in config.toml only? Is there some flag/option in file picker during file picking itself? |
I am personally very interested in the potential direction of making the file picker more interactive (change views/config, sort, move through directories or even tree-type views) as well as more informative (metadata). I would love to have a robust discussion of this, here or elsewhere, see what community interest is as well as compatibility with the vision for helix. |
@dannasessha I also had some thoughts about formatting in the picker, it's best to open an issue or discussion to discuss properly. |
I think #1163 is the main spot right for talk about the file picker now! |
see #10221 for current discussion |
Fixes #280
Connects settings from user defined config in
cargo.toml
to file picker by adding a second parameter tofile_picker()
.This is an extensible pattern for more user defined file picker configuration options, including the option to see or ignore dotfiles, etc.
Currently sets config for file picker to hide or display files listed in
.gitignore
.Sets default preference to see files listed in
.gitignore
.