Skip to content

Remember plot settings for each table - #819

Merged
MKleusberg merged 2 commits into
masterfrom
storeplot
Oct 14, 2016
Merged

MKleusberg merged 2 commits into
masterfrom
storeplot

Conversation

@MKleusberg

Copy link
Copy Markdown
Member

Store and restore the plot settings for each table - just like the
filter values or the column widths. This way you can have multiple
plots, one for each table.

Also, save these settings in the project files.

Store and restore the plot settings for each table - just like the
filter values or the column widths. This way you can have multiple
plots, one for each table.

Also, save these settings in the project files.
@MKleusberg

MKleusberg commented Oct 12, 2016 •

Copy link
Copy Markdown
Member Author

Putting this in a PR for a quick review and Travis check because of Qt4. The plot stuff is a little chaotic but I tried to keep the changes to it minimal. Might need a rework some day though. Anyway, maybe somebody wants to do some testing of this - might be that I've forgotten to save some combination of settings or whatever.
If it compiles on the other hand and doesn't break too badly for the first one or two tests, this should be ready to get merged. It can only improve things :)

@MKleusberg

Copy link
Copy Markdown
Member Author

Oh I forgot: this PR is related to #816.

@MKleusberg MKleusberg mentioned this pull request Oct 12, 2016
5 of 13 tasks
@revolter

Copy link
Copy Markdown
Member

Where and how are these saved?

@MKleusberg

Copy link
Copy Markdown
Member Author

There is a data structure called BrowseDataTableSettings (see https://github.com/sqlitebrowser/sqlitebrowser/blob/storeplot/src/MainWindow.h#L60) where all these sort of things are stored in. This PR adds some more entries to it for storing the plot information. We have a map (here https://github.com/sqlitebrowser/sqlitebrowser/blob/storeplot/src/MainWindow.h#L163) which maps one table to one of these structs, so there is an instance of this struct for each table. Now the PR adds code here and there to access the new attributes: store the updated information the map when the user changes some settings, get the stored information from the map when the user changes the current table.

This map is just stored as-is in the project files. Here's the loading code (https://github.com/sqlitebrowser/sqlitebrowser/blob/storeplot/src/MainWindow.cpp#L2237) and here's the saving code (https://github.com/sqlitebrowser/sqlitebrowser/blob/storeplot/src/MainWindow.cpp#L234). These lines didn't need to be changed for this. But you'll see we use the operator<< and operator>> there. These operators are overloaded here (https://github.com/sqlitebrowser/sqlitebrowser/blob/storeplot/src/MainWindow.h#L72). This PR adds two lines to each of these functions to store the new attributes in the struct to the project files or load it from the project files.
So in short, as long as the information is stored in the browseTableSettings map, all you need to do update the operator>> and operator<< functions and it'll also be written to the project files. If you open In the project files, you'll see that the stored map is just an unreadable mess but whatever.

Hope this explanation wasn't to long or too unstructured to be understood 😃

@justinclift

Copy link
Copy Markdown
Member

Hope this explanation wasn't to long or too unstructured to be understood

It seems like a really well written to me. 😄

Maybe we should cut-n-paste stuff like this into a wiki page for other developers too?

@MKleusberg

Copy link
Copy Markdown
Member Author

Yeah, sure. Might be hard to keep these things up-to-date but we can always delete explanations like these when we feel they do more harm than good.

@revolter

Copy link
Copy Markdown
Member

Great explanation! Yeah, I think this should be in a wiki too, though with more class/struct/method names inline than links to lines of code, as they will change over time.

@MKleusberg

Copy link
Copy Markdown
Member Author

@revolter Good thinking, that an even better solution! And at least this piece of code should be more or less stable for the next time.

Does somebody volunteer to start the Wiki page? I could do some proof reading afterwards.

@justinclift

Copy link
Copy Markdown
Member

@chrisjlocke @glawrence Any interest in helping with #819 (comment)? 😄

@revolter

Copy link
Copy Markdown
Member

Started something here: https://github.com/sqlitebrowser/sqlitebrowser/wiki/Source-code-highlights. Feedback welcomed 😄

@MKleusberg

Copy link
Copy Markdown
Member Author

Somebody tested the commit itself already? 😺

@MKleusberg

Copy link
Copy Markdown
Member Author

Just updated the commit a bit to make absolutely sure we're backward compatible with project files from older versions of DB4S. So if the Travis check doesn't fail, I'm just going to proactively merge this 😃 It really shouldn't make things any worse 😉

@MKleusberg
MKleusberg merged commit d5c6b7b into master Oct 14, 2016
@MKleusberg
MKleusberg deleted the storeplot branch October 14, 2016 11:24
@revolter

Copy link
Copy Markdown
Member

@MKleusberg, Never used project files, if I remember to test this, I will do so. If not, our users will test in when it's live 😆

@MKleusberg

Copy link
Copy Markdown
Member Author

To be honest, I don't use them either 😉 But some real life testing won't hurt anyway.

@vtronko

vtronko commented Oct 30, 2016 •

Copy link
Copy Markdown
Member

-'member plot settings for each table?
-Yeah I 'member!

memberberries

(couldn't help myself not to post this 😄 )

@justinclift

Copy link
Copy Markdown
Member

Someone's been watching South Park. 😁

vtronko pushed a commit that referenced this pull request Dec 29, 2016
Store and restore the plot settings for each table - just like the
filter values or the column widths. This way you can have multiple
plots, one for each table.

Also, save these settings in the project files.
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.

4 participants