Allow viewing via the EditDialog for executed sql statements #570

larsimmisch · 2016-04-20T19:19:51Z

This patch allows to view result data from the Execute SQL tab in the EditDialog.

This patch is lightweight and already useful to me - and hopefully other people

Ideally, I'd like to be able to edit the data too, but I couldn't get it to work, because when the user enters freeform SQL, it is nontrivial to determine what the underlying table is (see http://www.sqlite.org/sessions/lang_select.html).

Editing is disallowed, becuase we do not know which table the freeform query operates on

justinclift · 2016-04-20T21:29:08Z

src/SqlExecutionArea.h

@@ -32,7 +34,8 @@ class SqlExecutionArea : public QWidget
    SqliteTableModel* getModel() { return model; }
    QTextEdit* getResultView();
    SqlTextEdit* getEditor();
-
+	ExtendedTableWidget *getTableResult();


Hmmm, this seems to be using embedded tabs instead of spaces.

Would you be ok to fix that (for the other files you're touching too)? 😄

Ah, sorry. These are the (retarded) whitespace settings I had to configure for work.

Should be fixed

justinclift · 2016-04-20T21:51:59Z

Thanks, trying it out here now. 😄

justinclift · 2016-04-20T21:55:11Z

Hmmm, I must be blind... I'm not noticing a change from an end user point of view?

Er.... clue me in? 😀

justinclift · 2016-04-20T22:12:36Z

src/MainWindow.cpp

@@ -989,7 +989,8 @@ void MainWindow::mainTabSelected(int tabindex)
    } else if(tabindex == 3) {
        SqlExecutionArea* sqlWidget = qobject_cast<SqlExecutionArea*>(ui->tabSqlAreas->currentWidget());

-        m_currentTabTableModel = sqlWidget->getModel();
+        if (sqlWidget)


Got me. It's late.

larsimmisch · 2016-04-20T22:16:09Z

Ah. I think there is a double-click action missing, so I have to admit it is nonobvious:

To see what I am trying to accomplish:

open a database
enable View -> Edit Cell
Go to Execute SQL and enter a select statement suitable for your db.
In the result table view, click on an item and watch the Edit Cell update the data.

I use this for a db that has lots of JSON blobs that I need to inspect.

justinclift · 2016-04-20T22:39:04Z

Ahhh, interesting addition. Found a bug in it though, where it's not clearing the existing Edit Cell data when a new field on the same row is clicked.

The test database I'm using is very small (516KB). Just emailed it to you, along with the SQL used to trigger the bug. Hopefully it's something simple. 😄

larsimmisch · 2016-04-20T22:48:26Z

Yes, please.

Also, I'll look into adding a double click action so that the Edit Dialog opens when a cell is double clicked. That would make the new functionality much more discoverable.

(But not today, I'll look into it tomorrow)

justinclift · 2016-04-20T22:49:47Z

Sure, no worries at all. It's late here for me too. 😄

larsimmisch · 2016-04-21T08:09:08Z

I added the double click action (much better).

But I can not reproduce your bug on my machine. In any case, I don't understand how I could have introduced that sort of bug - this pull request mostly only connects the edit dialog with the result table.

Can you reproduce your bug when browsing the table?

justinclift · 2016-04-21T13:33:13Z

Cool. 😄

For the weird bug behaviour, @MKleusberg returned from holidays yesterday, so he's probably the right person to review bits. (there is a large queue for him through 😵 )

I really should get around to learning C++ eh? 😉

MKleusberg · 2016-04-23T21:58:25Z

Hi Lars!
First of all thank you very much for the contribution 😃 I've just had a first quick look and it seems to work very well and to really improve the user experience 👍 I'll have a closer look soon and will just put some comments into the Github diff. From what I can tell it'll be mostly code style though 😃

I'm not sure either how the bug mentioned by Justin could be introduced by this. But maybe he can help us out here debugging this - though I assume the bug is to be found somewhere else, most likely in my code for the edit cell panel itself, not in your code. So I probably won't be worrying about that too much for this pull request.

larsimmisch · 2016-04-23T23:03:41Z

Thanks for the quick thumbs-up (to both of you)

If you have suggestions regarding code style, do let me know and I will update the pull request

MKleusberg · 2016-04-24T22:05:13Z

src/ExtendedTableWidget.cpp

@@ -270,7 +270,8 @@ QSet<int> ExtendedTableWidget::selectedCols()
 void ExtendedTableWidget::cellClicked(const QModelIndex& index)
 {
    // If Alt key is pressed try to jump to the row referenced by the foreign key of the clicked cell
-    if(qApp->keyboardModifiers().testFlag(Qt::ControlModifier) && qApp->keyboardModifiers().testFlag(Qt::ShiftModifier) && model())
+    if(qApp->keyboardModifiers().testFlag(Qt::ControlModifier) && qApp->keyboardModifiers().testFlag(Qt::ShiftModifier)
+       && model())


I'd prefer to keep this line as it was. Since there is no code changed in this file, changing the formatting of this line could be confusing later: it looks like the code in this file was changed by this commit when in reality it wasn't. Hope this makes sense 😀

Sorry - that was just an automatism on my part. I'll revert that change

MKleusberg · 2016-04-24T22:30:12Z

I've put some comments concerning the code-style in your commits. Nothing major, though 😃 There's one more techical suggestion in there, too. Despite those I just noticed one more thing: if you have multiple SQL tabs opened, each showing a query result with a selected cell, the edit cell panel isn't updated when you switch between the tabs. This might be a bit confusing for the user to see the 'old' value there. But again, nothing major 😀 If you're interested you could maybe add a new slot which is getting triggered when the selected SQL tab changes and which just updates the panel to the current selection.

Sorry again for the nitpicking parts 😃 I actually think this is quite a good pull request which I hope we can merge as soon as possible 👍

…Kleusberg

…Kleusberg

larsimmisch · 2016-04-25T11:27:21Z

Hi @MKleusberg: I've made the changes you suggested - thank you for your thorough review.

I'll look into the new slot for multiple SQL tabs this evening, but I'd prefer to open a new pull request for that.

MKleusberg · 2016-04-25T22:16:25Z

Awesome! Looks good 👍 Thank you very much for this 😃 If you should need any help with your next pull request feel free to always open an issue or email me directly - I'd be glad to help out.

Allow viewing via the EditDialog for executed sql statements

4726bbf

Editing is disallowed, becuase we do not know which table the freeform query operates on

justinclift reviewed Apr 20, 2016
View reviewed changes

Fix whitespace

7c3b78c

Don't crash when changing to 'Execute SQL' tab when no database selected

8db6a18

justinclift reviewed Apr 20, 2016
View reviewed changes

Add double click action

844009a

justinclift assigned MKleusberg Apr 23, 2016

justinclift added this to the 3.9.0 - Something Nifty Treads Here Lightly milestone Apr 23, 2016

justinclift added the enhancement Feature requests. label Apr 23, 2016

MKleusberg reviewed Apr 24, 2016
View reviewed changes

A small usability improvement and indentation changes suggested by @M…

7ff9fe9

…Kleusberg

MKleusberg merged commit 1192f45 into sqlitebrowser:master Apr 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow viewing via the EditDialog for executed sql statements #570

Allow viewing via the EditDialog for executed sql statements #570

larsimmisch commented Apr 20, 2016

justinclift Apr 20, 2016 •

edited

Loading

larsimmisch Apr 20, 2016

justinclift commented Apr 20, 2016

justinclift commented Apr 20, 2016

justinclift Apr 20, 2016

larsimmisch Apr 20, 2016

larsimmisch commented Apr 20, 2016

justinclift commented Apr 20, 2016 •

edited

Loading

larsimmisch commented Apr 20, 2016 •

edited

Loading

justinclift commented Apr 20, 2016

larsimmisch commented Apr 21, 2016

justinclift commented Apr 21, 2016

MKleusberg commented Apr 23, 2016

larsimmisch commented Apr 23, 2016

MKleusberg Apr 24, 2016

larsimmisch Apr 25, 2016

MKleusberg commented Apr 24, 2016

larsimmisch commented Apr 25, 2016

MKleusberg commented Apr 25, 2016

Allow viewing via the EditDialog for executed sql statements #570

Allow viewing via the EditDialog for executed sql statements #570

Conversation

larsimmisch commented Apr 20, 2016

justinclift Apr 20, 2016 • edited Loading

Choose a reason for hiding this comment

larsimmisch Apr 20, 2016

Choose a reason for hiding this comment

justinclift commented Apr 20, 2016

justinclift commented Apr 20, 2016

justinclift Apr 20, 2016

Choose a reason for hiding this comment

larsimmisch Apr 20, 2016

Choose a reason for hiding this comment

larsimmisch commented Apr 20, 2016

justinclift commented Apr 20, 2016 • edited Loading

larsimmisch commented Apr 20, 2016 • edited Loading

justinclift commented Apr 20, 2016

larsimmisch commented Apr 21, 2016

justinclift commented Apr 21, 2016

MKleusberg commented Apr 23, 2016

larsimmisch commented Apr 23, 2016

MKleusberg Apr 24, 2016

Choose a reason for hiding this comment

larsimmisch Apr 25, 2016

Choose a reason for hiding this comment

MKleusberg commented Apr 24, 2016

larsimmisch commented Apr 25, 2016

MKleusberg commented Apr 25, 2016

justinclift Apr 20, 2016 •

edited

Loading

justinclift commented Apr 20, 2016 •

edited

Loading

larsimmisch commented Apr 20, 2016 •

edited

Loading