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 "Seek N mins backward" and "Seek N mins forward" #40

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

Dolso
Copy link
Contributor

@Dolso Dolso commented Jan 15, 2024

Hello!

Problem

The already existing "Seek 10 mins backward" and "Seek 10 mins forward" may not be enough, because sometimes you need to get data from query_log that was recorded several hours ago. So you have to press the same button a lot in a row, which is not very convenient

Solution

As a solution to the problem, I propose to add 2 actions where we will shift not by 10 minutes, but by user entered value. That said, I find "Seek 10 mins backward" and "Seek 10 mins forward" useful too, because you don't always need to shift the period by a large value

Disadvantages of the solution

  • large number of actions are possible
  • inconvenient Alt + T and Alt + t button combinations
  • Perhaps instead of two actions, they should have added just one "Seek N mins" where the forward or backward shift was adjusted with a sign. Or an action where we would enter the datetime of the end of the period

Code deficiencies

I apologize in advance for my code. I know very little about Rust

  • possibly bad error handling, while getting a non integer number
  • maybe there is a better way to take out the duplicate code, besides the create_update_end_time_cb function

Thank you in advance!

Copy link
Owner

@azat azat left a comment

Choose a reason for hiding this comment

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

Hi Andrei, the idea LGTM and seeking specific number of minutes is very useful, though tons of bindings is questionable, but anyway I don't have better idea now, so I will merge the code as-is, and will think about better idea later.

The already existing "Seek 10 mins backward" and "Seek 10 mins forward"
may not be enough, because sometimes you need to get data from query_log
that was recorded several hours ago. So you have to press the same
button a lot in a row, which is not very convenient
@azat azat force-pushed the seek_n_mins_backward_forward branch from 4a04b5d to 0e5df7b Compare January 17, 2024 21:08
@azat azat merged commit 0e5df7b into azat:main Jan 17, 2024
5 checks passed
@azat
Copy link
Owner

azat commented Jan 25, 2024

@Dolso I was thinking about this more, and I think at least it should accept time range (date begin, end), not number of minutes, since usually you have time frame

@Dolso
Copy link
Contributor Author

Dolso commented Jan 25, 2024

@Dolso I was thinking about this more, and I think at least it should accept time range (date begin, end), not number of minutes, since usually you have time frame

Ok, I can try to implement later if it's not urgent

@azat
Copy link
Owner

azat commented Jan 25, 2024

Ok, I can try to implement later if it's not urgent

It is not, thank you!

@Dolso
Copy link
Contributor Author

Dolso commented Jan 28, 2024

I made a separate pull request: #42

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