jorgerojas26 / lazysql

A cross-platform TUI database management tool written in Go.
MIT License
916 stars 51 forks source link

[feat]: Refresh Current Table #97

Closed Biowulf21 closed 3 weeks ago

Biowulf21 commented 2 months ago

Hi! First off, I wanted to thank you guys for making LazySql. I've been trying to find a lightweight, TUI that lets me do SQL queries for quite a while now, and since I use LazyGit, this program is a no-brainer!

While the program works quite well, I find it hard to see the updated data inside my table (i.e after I write some data on my application) as there seems to be no way to refresh my currently focused table. All GUI applications that I've used before has this feature, and I think it would be a neat addition to LazySql

Thank you and keep up the good work! 👍

jorgerojas26 commented 2 months ago

Hi, thanks for the suggestion !

Yes, that definitely is a planned feature, i think it should be easy to implement. I'll take a look and make a PR.

JKleinne commented 1 month ago

+1

Current workaround is just reloading the table via the tree, but it would be nice if we can do <C-R> to manually refresh the table

pro0o commented 1 month ago

hi @jorgerojas26, is the feat implemented? if not, I'd be happy to work on it. also, Any guidance would be much appreciated!

jorgerojas26 commented 1 month ago

hi @jorgerojas26, is the feat implemented? if not, I'd be happy to work on it. also, Any guidance would be much appreciated!

Hi.

It's not been implemented yet. If you want to work on it, go head, any help is appreciated.

pro0o commented 1 month ago

Thanks, maybe you can assign me.

pro0o commented 1 month ago
// Refresh the currentTab
func (home *Home) TabRefresh() {
    tab := home.TabbedPane.GetCurrentTab()
    var table *ResultsTable

    if tab != nil {
        table = tab.Content
    }

    if table != nil && !table.GetIsEditing() && !table.GetIsFiltering() {
        results := table.FetchRecords(func() {
            home.focusLeftWrapper()
        })

        // reflect multiple changes on sidebar
        if len(results) > 1 && !table.GetShowSidebar() {
            table.ShowSidebar(true)
        }

        if table.state.error == "" {
            home.focusRightWrapper()
        }

        app.App.ForceDraw()
    }
}
// cmd capture
switch command {
    case commands.TabRefresh:
        home.TabRefresh()
        return nil

With these addition in Home.go, refresh feat is working super fine, though idk how much optimal it will be for very very large db. Any suggestions is welcomed. @jorgerojas26

pro0o commented 1 month ago

Also what key binding should i give to refresh: <C-R> or <S-R>

jorgerojas26 commented 1 month ago
// Refresh the currentTab
func (home *Home) TabRefresh() {
  tab := home.TabbedPane.GetCurrentTab()
  var table *ResultsTable

  if tab != nil {
      table = tab.Content
  }

  if table != nil && !table.GetIsEditing() && !table.GetIsFiltering() {
      results := table.FetchRecords(func() {
          home.focusLeftWrapper()
      })

      // reflect multiple changes on sidebar
      if len(results) > 1 && !table.GetShowSidebar() {
          table.ShowSidebar(true)
      }

      if table.state.error == "" {
          home.focusRightWrapper()
      }

      app.App.ForceDraw()
  }
}
// cmd capture
switch command {
  case commands.TabRefresh:
      home.TabRefresh()
      return nil

With these addition in Home.go, refresh feat is working super fine, though idk how much optimal it will be for very very large db. Any suggestions is welcomed. @jorgerojas26

Those changes seem good. My only questions are:

Does it not work without it?

pro0o commented 1 month ago

Those changes seem good. My only questions are:

* Why focus left wrapper on error?

* Why focus right wrapper on success?

Does it not work without it?

yes those are not needed. well, after few reconfig & testing, here's the fully functioning feat for refresh in Home.go:

case commands.Refresh:
        if table != nil && !table.GetIsEditing() && !table.GetIsFiltering() && home.FocusedWrapper == focusedWrapperRight {
            tab := home.TabbedPane.GetCurrentTab()
            table := tab.Content
            if table.Loading != nil {
                app.App.SetFocus(table.Loading)
            }
            defer func() {
            //If records are fetched in any other menus than records, 
            //it appears that the menu section is not refreshed.
            //but the datas are refreshed and redrawn to records menu.
            //thats why 1st menu(records) is selected.
                table.Menu.SetSelectedOption(1)
                app.App.SetFocus(table)
            }()
            if err := table.FetchRecords(nil); err != nil {
                return event
            }
        }
    }

also i've given <S-R> as keybinding for refresh.

jorgerojas26 commented 4 weeks ago

Those changes seem good. My only questions are:

* Why focus left wrapper on error?

* Why focus right wrapper on success?

Does it not work without it?

yes those are not needed. well, after few reconfig & testing, here's the fully functioning feat for refresh in Home.go:

case commands.Refresh:
      if table != nil && !table.GetIsEditing() && !table.GetIsFiltering() && home.FocusedWrapper == focusedWrapperRight {
          tab := home.TabbedPane.GetCurrentTab()
          table := tab.Content
          if table.Loading != nil {
              app.App.SetFocus(table.Loading)
          }
          defer func() {
          //If records are fetched in any other menus than records, 
          //it appears that the menu section is not refreshed.
          //but the datas are refreshed and redrawn to records menu.
          //thats why 1st menu(records) is selected.
              table.Menu.SetSelectedOption(1)
              app.App.SetFocus(table)
          }()
          if err := table.FetchRecords(nil); err != nil {
              return event
          }
      }
  }

also i've given <S-R> as keybinding for refresh.

What do you think about implementing that feature directly in ResultsTable.go? the code could be smaller and i think it makes sense.

Other than that i think it's okay, can you can open a PR?

pro0o commented 4 weeks ago

What do you think about implementing that feature directly in ResultsTable.go? the code could be smaller and i think it makes sense.

yep it does make sense, my initial thought process was to somehow align this feature with tabs funcs, but refresh feat seems to more relate with menus.

Other than that i think it's okay, can you can open a PR?

for sure, i'll test and PR this feat. also are there any tests i can run?