os-autoinst / openqa-mon

Collection of terminal monitoring utilities for openQA
12 stars 13 forks source link

Improve pagination for openqa-mon #160

Closed grisu48 closed 3 months ago

grisu48 commented 3 months ago

Add some minor improvements to the pagination feature:

Follow-up of https://github.com/os-autoinst/openqa-mon/pull/157, in preparation of a Release-Candidate

grisu48 commented 3 months ago

@ilmanzo please have a look, if you have time :slightly_smiling_face:

ilmanzo commented 3 months ago

Overall very good, left only a small comment for better readability :)

ilmanzo commented 3 months ago

Nit: would be great to avoid a screen refresh/update when the page content does not actually change.

For example when you are on the first page and press Left or page up, then the screen flickers to update itself but it's really not needed. Maybe we can just use a dirty flag to signal the Update() function it's time to redraw or not ?

func (tui *TUI) PrevPage() {
    tui.Model.mutex.Lock()
    defer tui.Model.mutex.Unlock()
    if tui.currentPage > 0 {
      tui.currentPage--
      // here we need to refresh
      tui.dirty=true
    }
}

but we can just bring this to another issue, possibly related also to #159

grisu48 commented 3 months ago

Nit: would be great to avoid a screen refresh/update when the page content does not actually change.

For example when you are on the first page and press Left or page up, then the screen flickers to update itself but it's really not needed. Maybe we can just use a dirty flag to signal the Update() function it's time to redraw or not ?

func (tui *TUI) PrevPage() {
  tui.Model.mutex.Lock()
  defer tui.Model.mutex.Unlock()
    if tui.currentPage > 0 {
    tui.currentPage--
      // here we need to refresh
      tui.dirty=true
    }
}

but we can just bring this to another issue, possibly related also to #159

We can, but so far screen refreshes are cheap on a local terminal, so I didn't bothered much.