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

[Feature Request-Execute SQL] Allow bottom editor of Execute SQL tab to grow taller. #1670

Open
sky5walk opened this Issue Dec 17, 2018 · 21 comments

Comments

Projects
None yet
4 participants
@sky5walk
Copy link

sky5walk commented Dec 17, 2018

While debugging my SQL queries, I seem to make a lot of mistakes.
Thankfully, the errors are described in the bottom editor gadget.
But, the height of this editor is limited to only a few lines and I'm forced to constantly click and scroll to the bottom on each iteration of my edit. :((
Please allow this editor to grow to the full height of the window.


@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Dec 17, 2018

Yes, I think we can let that grow to the maximum height allowed by the minimum height of the other two panels. Maybe we used to write less information there and it was considered senseless, but I think it can be changed now without paying any price.

Note that you can also use the SQL Log pane to get the history of all the SQL executions done by the user (erroneous or not).

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Dec 17, 2018

Cool, yes, the Execute SQL results editor always has more information forcing a scroll to bottom with each execution.
I rarely use the SQL log since that accumulates all SQL actions. It is important, but only when something is really really wrong!

@mgrojo mgrojo self-assigned this Dec 18, 2018

mgrojo added a commit that referenced this issue Dec 18, 2018

Change the maximum size of the SQL results pane to a more flexible ap…
…proach

Set a default equal to the previous maximum, but allow user to grow this
pane to any value, permitted by the other widgets.

See issue #1670
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Dec 18, 2018

@sky5walk It's now as suggested and you'll have it in tomorrow's nightly. I haven't seen any problem.

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Dec 18, 2018

Awesome! I will send the scrollbar on vacation. :)

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Dec 19, 2018

Heh Heh Heh.

Kind of suspecting we'll be following up the 3.11.x series with a 3.12.0 release not too long after. 😉

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Dec 19, 2018

Just tried the extended Execute SQL feedback editor and it works great.
I can probably avoid scrolling for 80% of my queries.
This is a good compromise as I prefer more time spent on a future query builder... 😈

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Dec 20, 2018

Sorry, to ask after closing this topic...
I thought of another way to avoid scrolling so often.
Is it easy to place the last result line at the top of the results window?

Example query: (Imagine it is much longer than this 😈 )

FROM
Execution finished without errors.

-- At line 11:
INSERT INTO T (a, b, c, d)
VALUES
  (3, 1, 42, -7),
  (3, 2, 43, -8),
  (3, 3, 44, -9);
-- Result: query executed successfully. Took 0ms, 3 rows affected
TO
-- Result: query executed successfully. Took 0ms, 3 rows affected

-- At line 11:
INSERT INTO T (a, b, c, d)
VALUES
  (3, 1, 42, -7),
  (3, 2, 43, -8),
  (3, 3, 44, -9);
Execution finished without errors.
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 12, 2019

Is it easy to place the last result line at the top of the results window?

@mgrojo What do you reckon? 😄

@beRto-

This comment has been minimized.

Copy link

beRto- commented Jan 12, 2019

I like this suggestion, but would only suggest that "Execution finished without errors." also should come before the SQL. Knowing there were no errors is also new (and very useful) information when a query completes.

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Jan 12, 2019

My assumption was "Execution finished without errors." is redundant when the top states:
"-- Result: query executed successfully. Took 0ms, 3 rows affected"

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Jan 12, 2019

My assumption was "Execution finished without errors." is redundant when the top states:
"-- Result: query executed successfully. Took 0ms, 3 rows affected"

Yes, I agree. In fact I don't know why @MKleusberg added the phrases "Execution finished with(out) errors." in #1575. Maybe because some result messages aren't so clear as "query executed successfully" although the background colour was also added for helping on that regard.

My only concern about changing the order to:

-- Result: query executed successfully. Took 0ms, 3 rows affected
-- At line 11:
INSERT INTO T (a, b, c, d)
VALUES
  (3, 1, 42, -7),
  (3, 2, 43, -8),
  (3, 3, 44, -9);

is that the same message goes to the SQL Log with all the executed statements. I understand the log more easily if the query precedes the result. I'm unsure also, about whether both messages (log and results panel) should differ or be the same.

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Jan 12, 2019

Wow, thanks for the history...
I would strongly suggest dropping the whole results log since we already have the results window to view.
As you mention, the SQL log is redundantly populated!!
Reduce the clutter and continue to report to the SQL log,
but ONLY report to the results grid/editor.
SUCCESS = populate results grid.
FAIL/ERROR = populate results grid. <-- since the results grid will be empty in this case.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 13, 2019

@sky5walk That's confusing for me, as it seems to be contradicting itself? Is there a typo in it somewhere?

I would strongly suggest dropping the whole results log since we already have the results window to view.
...
Reduce the clutter and continue to report to the SQL log ...

@sky5walk

This comment has been minimized.

Copy link
Author

sky5walk commented Jan 13, 2019

Yeah, I don't code with qt lib, so I don't know the gadget class names?
Summary:
mgrojo stated the "sql log" captures the "execute sql" activity already.
I'm suggesting to drop the 3rd bottom editor in the execute sql panel entirely.
Just output successful queries to the 2nd grid
And ..
Output failed queries to the same grid.
This removes the whole scrolling and partitioning of 3 elements that by themselves are never tall enough.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 14, 2019

Thanks. Understanding has occurred. 😄

@beRto-

This comment has been minimized.

Copy link

beRto- commented Feb 10, 2019

@justinclift looks like this didn't make it into 3.11? :( Is there an ETA?

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 10, 2019

Some pieces are already in our nightly builds:

    https://nightlies.sqlitebrowser.org/latest/

Those will be in 3.12.0 - our next major release - which is probably only a few months away. We had a bunch of technical debt we needed to get through for the 3.11.0 release, which is why it took so long for that one. In theory (!) our next releases shouldn't be so far apart. 😄

@beRto-

This comment has been minimized.

Copy link

beRto- commented Feb 10, 2019

:(

Oh, I was hoping this was a minor 3.11.1 regression fix!

Previous db4s put the important query result info (number of rows and execution time) front and centre. So I thought it would be simpler.

Anyways, thanks for considering. And congrats on the new release!

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 10, 2019

@mgrojo Is this patch suitable for adding to the v3.11.x branch?

mgrojo added a commit that referenced this issue Feb 13, 2019

Change the maximum size of the SQL results pane to a more flexible ap…
…proach

Set a default equal to the previous maximum, but allow user to grow this
pane to any value, permitted by the other widgets.

See issue #1670
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Feb 13, 2019

Added to the v3.11.x branch.

@justinclift justinclift referenced this issue Feb 16, 2019

Closed

3.11.1 - outstanding pieces #1747

12 of 13 tasks complete
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 18, 2019

This still has stuff needing to be done, yeah? eg Don't close it yet?

mgrojo added a commit that referenced this issue Feb 23, 2019

Move query in results panel of Execute SQL to the bottom
This gives precedence to the results of the query. It also removes the
comment characters that only made sense in the SQL User log. The text is
splitted so the texts are only translated once. The output of a multi-query
execution is also improved, because the results and line are constant
while the query part is quickly changing.

After the execution of a multi-query text, the final text is not separated
by a blank line so the result message is visible in the second.

For users wanting more space in the editor and query table, the results
panel is now collapsible, since the SQL Log can be used instead.

See issues #1709 and #1670.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.