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

Add TUI, resolve #19, #17, #16 #21

Merged
merged 4 commits into from
Mar 20, 2021
Merged

Add TUI, resolve #19, #17, #16 #21

merged 4 commits into from
Mar 20, 2021

Conversation

ellie
Copy link
Member

@ellie ellie commented Mar 20, 2021

This totally removes the requirement for FZF! Also no longer lists all
the database contents and pipes them to something, and instead uses
SQLite for searching. Again, SQLite still needs optimizing! We're
literally using a LIKE query here, and something like the FTS module
would make more sense I think.

On top of this, at the moment the TUI is very... "initial
implementation". Ie, the code is a mess. It needs restructuring +
rethinking, but it works :) I'd like to include more stats in the TUI at
some point, without slowing it down too much!

Also automatically search for whatever the current command buffer is.

This totally removes the requirement for FZF! Also no longer lists all
the database contents and pipes them to something, and instead uses
SQLite for searching. Again, SQLite still needs optimizing! We're
literally using a LIKE query here, and something like the FTS module
would make more sense I think.

On top of this, at the moment the TUI is very... "initial
implementation". Ie, the code is a mess. It needs restructuring +
rethinking, but it works :) I'd like to include more stats in the TUI at
some point, without slowing it down too much!
@ellie ellie enabled auto-merge (squash) March 20, 2021 00:46
@ellie ellie disabled auto-merge March 20, 2021 00:46
@ellie ellie enabled auto-merge (squash) March 20, 2021 00:46
@ellie ellie merged commit 716c772 into main Mar 20, 2021
@ellie ellie deleted the add-tui branch March 20, 2021 00:50
Comment on lines +63 to +70
Some(i) => {
if i == 0 {
0
} else {
i - 1
}
}
None => 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Some(i) => {
if i == 0 {
0
} else {
i - 1
}
}
None => 0,
None | Some(0) => 0,
Some(i) => i - 1,

Comment on lines +176 to +181
if let Some(selected) = app.results_state.selected() {
if selected == i {
content.style =
Style::default().fg(Color::Red).add_modifier(Modifier::BOLD);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if let Some(selected) = app.results_state.selected() {
if selected == i {
content.style =
Style::default().fg(Color::Red).add_modifier(Modifier::BOLD);
}
}
if app.results_state.selected() == Some(i) {
content.style = Style::default().fg(Color::Red).add_modifier(Modifier::BOLD);
}

@bl-ue
Copy link
Contributor

bl-ue commented May 7, 2021

(off topic) @conradludgate how in the world did you approve after the PR was merged...?! 😄

@conradludgate
Copy link
Collaborator

(off topic) @conradludgate how in the world did you approve after the PR was merged...?! 😄

You can approve any PR post merge :)

Copy link
Contributor

@bl-ue bl-ue left a comment

Choose a reason for hiding this comment

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

Really! I can't, and have never been able to, even on repos that I have write access to, e.g. https://github.com/tldr-pages/tldr 🤔

@atuin-bot
Copy link

This pull request has been mentioned on Atuin Community. There might be relevant details there:

https://forum.atuin.sh/t/question-about-usage-of-stderr-stdout-in-atuin-search-i/66/2

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.

None yet

4 participants