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

Retain display formats #1202

Merged
merged 9 commits into from Nov 2, 2017

Conversation

Projects
None yet
3 participants
@mgrojo
Copy link
Contributor

mgrojo commented Oct 28, 2017

If the Colum Display Format Dialog was open again after setting a format, the combo box showed always the format as custom, instead of setting the correct value according to the SQL code. The dialog recognizes now the current format.

The display format items have been reorganized in groups with separators in between. This is because the complete alphabetical order looses sense in the translations, so it is better to group the items by type and then alphabetically.

mgrojo added some commits Oct 20, 2017

Added an option in the context menu for using the currently selected …
…cell

as filter in this column. This allows quick filtering by selected values.
Changes to pull request #1182 requested by @MKleusberg: get internal …
…cell data and check for the NULL special case.
Updated Spanish translation.
Reviewed translation following guidelines the KDE Spanish Translation Team: https://es.l10n.kde.org/
Warnings raised by linguist have been solved.
Fixed crashed when editing the display format and the currently brows…
…ed table name is actually a view.

Now it is checked the object type before the cast. This avoids the crash and the field name is obtained for each case.
The Colum Display Format Dialog recognizes now the current format in the
combo instead of falling back to Custom.

The display format items have been reorganized in groups with separators
in between. This is because the strict alphabetical order looses sense in
the translations.
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 28, 2017

Excellent, that looks like it'll be really useful. 😄

@MKleusberg
Copy link
Member

MKleusberg left a comment

Thanks again 👍 I've just found one cosmetic issue in the code. When that's fixed, this can be merged 😄

There were quite a few people complaining about the fact that the Display Format dialog would always show "Custom" even though you selected one from the list. So this is definitely a welcomed fix 😉

ui->comboDisplayFormat->insertSeparator(ui->comboDisplayFormat->count());
ui->comboDisplayFormat->addItem(tr("Custom"), "custom");
formatName = "custom";
}

This comment has been minimized.

@MKleusberg

MKleusberg Oct 30, 2017

Member

Just one minor thing: here and above in the foreach loop you're mixing spaces and tabs for indentation which makes the indentation look wrong if the tab width is not set to 8 spaces. Can you change the tabs to 4 spaces instead? 😄 By the way, if you're using Qt Creator you can go to the "Projects" panel (big button on the left), there click "Editor" in the "Project Settings" section, and then set the Tab policy to "Spaces only", the Tab size to "4" and the Indent Size to "4". That changes it just for the sqlitebrowser project and keeps the old settings for all your other projects. That's how I configured it, too 😉

This comment has been minimized.

@mgrojo

mgrojo Oct 31, 2017

Author Contributor

I wasn't aware that Emacs was inserting spaces. I've already changed its settings, so it no longer does that. I don't use Qt Creator for the moment, but I will give it a try.

This comment has been minimized.

@MKleusberg

MKleusberg Nov 2, 2017

Member

If you're used to Emacs, I'd stick with that (and I say that as a Vim-person 😉) but Qt Creator is definitely worth giving a try, especially when editing the .ui files 😄

This comment has been minimized.

@mgrojo

mgrojo Nov 2, 2017

Author Contributor

Some tasks are easier with those visual IDEs, but I know too many Emacs tricks and powerful commands for just abandoning it 😄

ui->comboDisplayFormat->addItem(tr("Custom"), "custom");
ui->comboDisplayFormat->setCurrentIndex(ui->comboDisplayFormat->findData("custom"));
QString formatName;
foreach (const QString &formatKey, formatFunctions.keys()) {

This comment has been minimized.

@MKleusberg

MKleusberg Oct 30, 2017

Member

You can also use a range-based for loop from C++11 here since we don't have to support older compilers anymore. The foreach provided by Qt is slower than the C++11 one because it creates a copy of the entire map before iterating over the copy. It's not a big deal though in this case.

This comment has been minimized.

@mgrojo

mgrojo Oct 31, 2017

Author Contributor

Perfect. I've committed the improvement.

Using a range-based for loop from C++11 standard instead of a
foreach loop from Qt.
Cleaned up some tabs inadvertently inserted as indentation.

@MKleusberg MKleusberg merged commit 31e499d into sqlitebrowser:master Nov 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Nov 2, 2017

Thanks 😄 And it's merged now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment