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

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

Conversation

kmod-midori
Copy link

This should make #85 eaiser to do, since we do not need to deal with the UI popping up. Bypassing the UI should also make mcfly start a little faster.

I was originally wanting to add support for - in output_selection, but since we are doing print! everywhere, this will not be effective as standard output may contain all kinds of messages.

@cantino
Copy link
Owner

cantino commented Aug 17, 2022

Thanks @chengyuhui. Can you explain a bit more what you hope to use this for?

@kmod-midori
Copy link
Author

I hope to have shell scripts call mcfly and get the result without the UI popping up. The linked issue provided a way to arrest the UI with the help of script, but that's not very elegant. In addition, this also removes the overhead of UI starting up, as I will be calling mcfly quite frequently.


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 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!

@cantino
Copy link
Owner

cantino commented Dec 10, 2022

Could do the proposal in #307, if it allowed a --limit on the number of returned matches, also meet this PR's goals?

@kmod-midori
Copy link
Author

Is it suitable for use when it is called on every key entered into the terminal? If it is, and it does not show the UI, then it should work well.

@cantino
Copy link
Owner

cantino commented Feb 26, 2023

Are you still working on this?

@plawley
Copy link

plawley commented Feb 22, 2024

just a bump that there is definitely interest in this no-ui direct suggestion capability.
-zsh-autosuggestions integration is a bit of a killer app for this type of tool.

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.

3 participants