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

File picker config #988

Merged
merged 18 commits into from
Nov 20, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions book/src/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 } |
Copy link
Member

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.

Copy link
Contributor Author

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!

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 the commit 80ba0e0 has the changes you were looking for!

All the pairs listed in the default configuration above are IgnoreOptions: which types of files are ignored in the file picker and global search.
`hidden` Directly enables ignoring hidden files.
`parents` Enables reading ignore files from parent directories.
`ignore` Enables reading `.ignore` files.
`git-ignore` Enables reading `.gitignore` files.
`git-global` Enables reading global .gitignore, whose path is specified in git's config: `core.excludefile` option.
`git-exclude` Enables reading `.git/info/exclude` files.
`max-depth` can also be used as a key, and can be bound to an integer value for maximum depth to recurse. Defaults to `None`.

## LSP

Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Application {
if first.is_dir() {
std::env::set_current_dir(&first)?;
editor.new_file(Action::VerticalSplit);
compositor.push(Box::new(ui::file_picker(".".into())));
compositor.push(Box::new(ui::file_picker(".".into(), &config.editor)));
} else {
let nr_of_files = args.files.len();
editor.open(first.to_path_buf(), Action::VerticalSplit)?;
Expand Down
79 changes: 47 additions & 32 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1359,6 +1359,7 @@ fn global_search(cx: &mut Context) {
let (all_matches_sx, all_matches_rx) =
tokio::sync::mpsc::unbounded_channel::<(usize, PathBuf)>();
let smart_case = cx.editor.config.smart_case;
let file_picker_config = cx.editor.config.file_picker.clone();

let completions = search_completions(cx, None);
let prompt = ui::regex_prompt(
Expand Down Expand Up @@ -1387,41 +1388,55 @@ fn global_search(cx: &mut Context) {

let search_root = std::env::current_dir()
.expect("Global search error: Failed to get current dir");
WalkBuilder::new(search_root).build_parallel().run(|| {
let mut searcher_cl = searcher.clone();
let matcher_cl = matcher.clone();
let all_matches_sx_cl = all_matches_sx.clone();
Box::new(move |dent: Result<DirEntry, ignore::Error>| -> WalkState {
let dent = match dent {
Ok(dent) => dent,
Err(_) => return WalkState::Continue,
};

match dent.file_type() {
Some(fi) => {
if !fi.is_file() {
return WalkState::Continue;
WalkBuilder::new(search_root)
.hidden(file_picker_config.hidden)
.parents(file_picker_config.parents)
.ignore(file_picker_config.ignore)
.git_ignore(file_picker_config.git_ignore)
.git_global(file_picker_config.git_global)
.git_exclude(file_picker_config.git_exclude)
.max_depth(file_picker_config.max_depth)
.build_parallel()
.run(|| {
let mut searcher_cl = searcher.clone();
let matcher_cl = matcher.clone();
let all_matches_sx_cl = all_matches_sx.clone();
Box::new(move |dent: Result<DirEntry, ignore::Error>| -> WalkState {
let dent = match dent {
Ok(dent) => dent,
Err(_) => return WalkState::Continue,
};

match dent.file_type() {
Some(fi) => {
if !fi.is_file() {
return WalkState::Continue;
}
}
None => return WalkState::Continue,
}
None => return WalkState::Continue,
}

let result_sink = sinks::UTF8(|line_num, _| {
match all_matches_sx_cl
.send((line_num as usize - 1, dent.path().to_path_buf()))
{
Ok(_) => Ok(true),
Err(_) => Ok(false),
let result_sink = sinks::UTF8(|line_num, _| {
match all_matches_sx_cl
.send((line_num as usize - 1, dent.path().to_path_buf()))
{
Ok(_) => Ok(true),
Err(_) => Ok(false),
}
Comment on lines +1420 to +1425
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

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

Here are the same lines in master.

});
let result =
searcher_cl.search_path(&matcher_cl, dent.path(), result_sink);

if let Err(err) = result {
log::error!(
"Global search error: {}, {}",
dent.path().display(),
err
);
}
});
let result = searcher_cl.search_path(&matcher_cl, dent.path(), result_sink);

if let Err(err) = result {
log::error!("Global search error: {}, {}", dent.path().display(), err);
}
WalkState::Continue
})
});
WalkState::Continue
})
});
} else {
// Otherwise do nothing
// log::warn!("Global Search Invalid Pattern")
Expand Down Expand Up @@ -2593,7 +2608,7 @@ fn command_mode(cx: &mut Context) {

fn file_picker(cx: &mut Context) {
let root = find_root(None).unwrap_or_else(|| PathBuf::from("./"));
let picker = ui::file_picker(root);
let picker = ui::file_picker(root, &cx.editor.config);
cx.push_layer(Box::new(picker));
}

Expand Down
11 changes: 10 additions & 1 deletion helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,22 @@ 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);
Copy link
Member

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.

Copy link
Contributor Author

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!

Copy link
Contributor Author

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.

walk_builder
.hidden(config.file_picker.hidden)
.parents(config.file_picker.parents)
.ignore(config.file_picker.ignore)
.git_ignore(config.file_picker.git_ignore)
.git_global(config.file_picker.git_global)
.git_exclude(config.file_picker.git_exclude)
.max_depth(config.file_picker.max_depth);

let walk_builder = match type_builder.add(
"compressed",
"*.{zip,gz,bz2,zst,lzo,sz,tgz,tbz2,lz,lz4,lzma,lzo,z,Z,xz,7z,rar,cab}",
Expand Down
40 changes: 40 additions & 0 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,42 @@ where
Ok(Duration::from_millis(millis))
}

#[derive(Debug, Clone, PartialEq, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct FilePickerConfig {
pub hidden: bool,
pub parents: bool,
pub ignore: bool,
pub git_ignore: bool,
pub git_global: bool,
pub git_exclude: bool,
pub max_depth: Option<usize>,
}

impl Default for FilePickerConfig {
fn default() -> Self {
Self {
// 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,
Copy link
Member

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.

Copy link
Contributor Author

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.

}
}
}

#[derive(Debug, Clone, PartialEq, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct Config {
Expand Down Expand Up @@ -61,6 +97,9 @@ pub struct Config {
pub completion_trigger_len: u8,
/// Whether to display infoboxes. Defaults to true.
pub auto_info: bool,
/// Whether to hide files in .gitignore from displaying in file picker. Defaults to false.
//pub git_ignore: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good.

pub file_picker: FilePickerConfig,
}

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
Expand Down Expand Up @@ -92,6 +131,7 @@ impl Default for Config {
idle_timeout: Duration::from_millis(400),
completion_trigger_len: 2,
auto_info: true,
file_picker: FilePickerConfig::default(),
}
}
}
Expand Down