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

no_ui option that directly writes the first suggestion to a file #276

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

no_ui option that directly writes the first suggestion to a file #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 35 additions & 0 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,41 @@ fn handle_addition(settings: &Settings) {

fn handle_search(settings: &Settings) {
let history = History::load(settings.history_format);

if settings.no_ui {
history.build_cache_table(
&settings.dir.to_owned(),
&Some(settings.session_id.to_owned()),
None,
None,
None,
settings.limit.to_owned(),
);
let matches = history.find_matches(
&settings.command,
settings.results as i16,
settings.fuzzy,
&mcfly::settings::ResultSort::Rank,
);

let cmd = match matches.first() {
Some(cmd) => cmd.cmd.to_owned(),
None => settings.command.to_owned(),
Copy link
Owner

Choose a reason for hiding this comment

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

It seems like if it cannot find a match, it should return nothing, not the search query?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I somehow misunderstood what the original UI code was doing.

Copy link
Owner

Choose a reason for hiding this comment

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

This is still returning the search query, right?

};
let cmd_t = cmd.trim();
if cmd_t.is_empty() {
return;
}

let out = format!("mode display\ncommandline {}\n", cmd_t);
if let Some(path) = &settings.output_selection {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of requiring --output_selection, you could allow two options:

  1. With --output_selection, write to the --output_selection selection file as you're doing now.
  2. Without output_selection, instead just print one or more matches to STDOUT for scripts to parse.

Copy link
Author

Choose a reason for hiding this comment

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

It was originally written this way, however I found that we are happily print!-ing many messages to STDOUT, which (given the current output format) might mess up parsing for simple scripts.

Copy link
Owner

Choose a reason for hiding this comment

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

We should only be using print! to output when there's an error. Those should probably be converted to use STDERR.

Copy link
Author

Choose a reason for hiding this comment

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

We should update all print! to eprint(ln)! before merging this.

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed!

fs::write(path, out)
.unwrap_or_else(|err| panic!("McFly error: unable to write to {}: {}", path, err));
}

return;
}

let result = Interface::new(settings, &history).display();
if let Some(cmd) = result.selection {
if let Some(path) = &settings.output_selection {
Expand Down
10 changes: 10 additions & 0 deletions src/settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub struct Settings {
pub interface_view: InterfaceView,
pub result_sort: ResultSort,
pub disable_menu: bool,
pub no_ui: bool,
}

impl Default for Settings {
Expand Down Expand Up @@ -116,6 +117,7 @@ impl Default for Settings {
interface_view: InterfaceView::Top,
result_sort: ResultSort::Rank,
disable_menu: false,
no_ui: false,
}
}
}
Expand Down Expand Up @@ -204,6 +206,9 @@ impl Settings {
.arg(Arg::with_name("delete_without_confirm")
.long("delete_without_confirm")
.help("Delete entry without confirm"))
.arg(Arg::with_name("no_ui")
.long("no-ui")
.help("Don't show the UI; requires --output-selection"))
.arg(Arg::with_name("output_selection")
.short("o")
.long("output-selection")
Expand Down Expand Up @@ -419,6 +424,11 @@ impl Settings {
settings.output_selection = search_matches
.value_of("output_selection")
.map(|s| s.to_owned());
settings.no_ui = search_matches.is_present("no_ui");

if settings.no_ui && settings.output_selection.is_none() {
panic!("McFly error: no_ui requires output_selection");
}

if let Some(values) = search_matches.values_of("command") {
settings.command = values.collect::<Vec<_>>().join(" ");
Expand Down