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

Show last run time for each command #149

Merged
merged 5 commits into from
Jun 6, 2021
Merged

Conversation

dmfay
Copy link
Contributor

@dmfay dmfay commented May 23, 2021

1621797105

with thanks to atuin for the idea of showing timing!

@@ -244,13 +245,27 @@ impl History {

like_query.push_str("%");

let query = "SELECT id, cmd, cmd_tpl, session_id, when_run, exit_code, selected, dir, rank,
let query = "WITH matched_commands AS (
SELECT id, cmd, cmd_tpl, session_id, when_run, exit_code, selected, dir, rank,
Copy link
Owner

Choose a reason for hiding this comment

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

Does using this when_run not work, such that you need to do the second query? If this isn't the most recent, maybe when the contextual_commands temp table is made, it should be MAX(when_run)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There would need to be at least two queries here to get the max grouped only by cmd but on looking temp table creation is a much better place for this!

@cantino
Copy link
Owner

cantino commented May 23, 2021

Could we make it use less space by just showing 5d or 30m or 10s. I don't think 30m 10s adds much?

@dmfay
Copy link
Contributor Author

dmfay commented May 23, 2021

I've liked having days-hours and hours-minutes resolution in my test-driving so far, so I may have sold it a bit short with the recent git commands. That said, other scales are definitely not as useful, so that specific band really has to be worth the four characters. Try it out for a day or three and see what you think, maybe?

@cantino
Copy link
Owner

cantino commented May 24, 2021

I've liked having days-hours and hours-minutes resolution in my test-driving so far, so I may have sold it a bit short with the recent git commands. That said, other scales are definitely not as useful, so that specific band really has to be worth the four characters. Try it out for a day or three and see what you think, maybe?

Ok!

@cantino
Copy link
Owner

cantino commented May 26, 2021

I don't mind the two part dates, but I am finding it a bit hard to parse when I skim the list. I wonder if we should make it a lighter color (on dark backgrounds) and put it flush to the right?

@dmfay
Copy link
Contributor Author

dmfay commented May 27, 2021

here's a version with timings on the right. The FixedLengthGraphemeString looks like it only checks on input and returns whatever the internal string has accumulated (even if shorter), though, and I'm not quite sure what to do about that. We have width but do we have the current cursor x somewhere?

@cantino
Copy link
Owner

cantino commented May 30, 2021

I'm not 100% sure what you're asking. If you need to know if the line is currently selected in truncate_for_display, you could pass in index == self.selection to it. Is that what you're asking? All FixedLengthGraphemeString does is ensure that you cannot add more than N unicode characters to the string.

Do you like how it looks if you put

write!(screen, "{}", cursor::Goto(width - 9, index as u16 + RESULTS_TOP_INDEX)).unwrap();

on line 248?

@dmfay
Copy link
Contributor Author

dmfay commented May 30, 2021

I was trying to fill the remaining width between command text and timing info with spaces and not having much luck. cursor::Goto does the trick, thanks!

@cantino
Copy link
Owner

cantino commented Jun 6, 2021

Thanks @dmfay, this is a nice addition :)

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.

2 participants