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

Retain display formats #1202

Merged
merged 9 commits into from Nov 2, 2017

Conversation

mgrojo
Copy link
Member

@mgrojo 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.

…cell

as filter in this column. This allows quick filtering by selected values.
…get internal cell data and check for the NULL special case.
Reviewed translation following guidelines the KDE Spanish Translation Team: https://es.l10n.kde.org/
Warnings raised by linguist have been solved.
…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.
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
Copy link
Member

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

Copy link
Member

@MKleusberg MKleusberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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";
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. I've committed the improvement.

foreach loop from Qt.
Cleaned up some tabs inadvertently inserted as indentation.
@MKleusberg MKleusberg merged commit 31e499d into sqlitebrowser:master Nov 2, 2017
@MKleusberg
Copy link
Member

Thanks 😄 And it's merged now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants