Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 8, 2020

Hello all,

Please review my first iteration of the Dutch (nl) translation for version v3.12.1.

Here's a few notes to go along with it as well as a few issues I encountered in the process:

  • General:

    • I took the liberty of adding a few extra accelerators here and there, but have not applied this consistently. In the next iteration I could either remove them, if you want, or add them more consistently.
    • Perhaps you are already aware of the following and it's something that is still under development:
      • Some buttons in dialogs, such as Cancel, OK and Restore Defaults, and in some context menu's items, such as Copy, Paste, etc., were not translatable and/or don't appear as translated. Perhaps some are provided by the system and cannot be overwritten? 1
      • Some translations for filtering file types in import and export dialogs do not appear, even though I did translate them. 1
      • Links in tooltips, such as <a href="https://en.wikibooks.org/wiki/Regular_Expressions">Regular Expression in Wikibooks</a>, don't appear to work (I've tested this in English as well, to make sure it wasn't caused by my translations). 1
  • SqlUiLexer:

    • I took the liberty of adding newlines in the tooltips for function documentation, because they were too long to be completely visible otherwise.
    • Tooltips with function documentation don't show Unicode (accented) characters, they appear as the infamous replacement character �. 1

I can provide more details about these issues, if desired.

Should I file some of these as issues to your issue tracker, perhaps?

Cheers!


  1. Tested on Ubuntu 18.04.5 (Linux kernel 4.15.0)

@lucydodo
Copy link
Member

lucydodo commented Oct 8, 2020

Thanks for contributing! 😄 First of all, you need to inform DB4S that there is a Dutch translation.
Please refer to the existing lines and add them to the following files:

  • ./CMakeLists.txt:303
  • ./src/src.pro:19:200
  • ./src/translations/flags/flags.qrc:21
  • ./src/translations/translations.qrc:19

I'm on mobile right now, so I simply made a review, but if you need more information, let me know. 🤔

@ghost
Copy link
Author

ghost commented Oct 8, 2020

@lucydodo You are most welcome! My pleasure to contribute to such a great project.

Thanks for the pointers. I had already changed the files you mentioned, for testing purposes, but I just wasn't sure about including those changes in the commits for the pull request. I figured perhaps you, the maintainers, would want to keep control over such final configurations. But, I'll gladly add those references myself. Shouldn't take too long.


edit:

Done!

Copy link
Member

@lucydodo lucydodo left a comment

Choose a reason for hiding this comment

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

I checked the changes. Overall, I've found a number of items that are perfect, but need simple fix. Please check them. 🤔
If you are uncomfortable because it is too much, you can only resolve the untranslated items* and I can solve the rest.
Proceed as you like. and really thank you so much for translating all of this! 😄 😄

*: Items in which the review message written No translation here, but it treated completed.*.

<message>
<location filename="../CondFormatManager.ui" line="35"/>
<source>&amp;Add</source>
<translation></translation>
Copy link
Member

Choose a reason for hiding this comment

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

No translation here, but it treated completed.

<message>
<location filename="../EditDialog.ui" line="83"/>
<source>JSON</source>
<translation></translation>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a proper noun that doesn't need translation, so I think you've left the translation blank.
I also did like this in the past, but there was an comments that it is safer to copy the original text for proper nouns
that do not require translation. Please refer to here for details.

<message>
<location filename="../EditDialog.ui" line="88"/>
<source>XML</source>
<translation></translation>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a proper noun that doesn't need translation, so I think you've left the translation blank.
I also did like this in the past, but there was an comments that it is safer to copy the original text for proper nouns
that do not require translation. Please refer to here for details.

<message>
<location filename="../ExportDataDialog.ui" line="102"/>
<source>,</source>
<translation></translation>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

<message>
<location filename="../ExportDataDialog.ui" line="107"/>
<source>;</source>
<translation></translation>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

</message>
<message>
<location filename="../FileDialog.h" line="38"/>
<source>JSON Files (*
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

</message>
<message>
<location filename="../FileDialog.h" line="38"/>
<source>JSON Files (*
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

</message>
<message>
<location filename="../FileDialog.h" line="38"/>
<source>JSON Files (*
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

</message>
<message>
<location filename="../FileDialog.h" line="38"/>
<source>JSON Files (*
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a symbol that doesn't need translation, so I think you've left the translation blank.
I also have a similar case in the past, but there was an comments that it is safer to copy the original text for symbol
that do not require translation. Please refer to here for details.

@ghost
Copy link
Author

ghost commented Oct 8, 2020

@lucydodo I think I've amended the translations file with all your requested changes. What's the correct course of action now? Should I simply commit and push again? I assume that that will erase all your requests for changes, and I'm not sure whether that is desirable. Please advice. Cheers.

@lucydodo
Copy link
Member

lucydodo commented Oct 8, 2020

Just commit and push and GitHub will be clean it up again :)

@lucydodo lucydodo merged commit 3041380 into sqlitebrowser:master Oct 8, 2020
@lucydodo
Copy link
Member

lucydodo commented Oct 8, 2020

Thanks for responding despite the many requests for modification. I modified and force push the following:

  • The tab spacing in CMakeLists.txt was previously 4, but it was set to 8, so I corrected for 4.
  • '...' item in PreferencesDialog context was not translated, so I replaced by original text.

Now squash and merged. I will cherry-pick in the v3.12.x branch soon.
Thanks to this PR, our project has taken a step further. Thank You! 😄

@lucydodo
Copy link
Member

lucydodo commented Oct 8, 2020

I took the liberty of adding newlines in the tooltips for function documentation,
because they were too long to be completely visible otherwise.

It doesn't problem you make a newline in the tooltip for readability.

I took the liberty of adding a few extra accelerators here and there, but have not applied this consistently.
In the next iteration I could either remove them, if you want, or add them more consistently.

I'm not sure if adding an accelerator not specified in the original code works.
I tried to test it, but none of the accelerators work on my Mac(even English env). I'll test it on a Windows PC.

Some buttons in dialogs, such as Cancel, OK and Restore Defaults, and in some context menu's items, such as Copy, Paste, etc., were not translatable and/or don't appear as translated.
Perhaps some are provided by the system and cannot be overwritten?

Cancel, OK and Restore Defaults, these buttons are text assigned through the QtStandardButtons provided by Qt.
So the text of this button we can NOT translate and Qt Team needs to translate it,
but I don't know why the Dutch language translation file is not in the Qt source tree, but it is confirmed that it is none.

Links in tooltips, such as Regular Expression in Wikibooks, don't appear to work

Tooltips with function documentation don't show Unicode (accented) characters,
they appear as the infamous replacement character

Could you please give me more information on the issue?

@ghost
Copy link
Author

ghost commented Oct 8, 2020

@lucydodo Haha, no worries — they were all minor changes that were easily fixed. And you're welcome! Glad to see everything went pretty fast and reasonably smooth! 👍

And apologies about the indention issue in CMakeLists.txt. I don't know why my editor chose to change the tab spaces. I'll make sure to check for that next time, before I make a commit.

Anyway, looking forward to see my translations in the upcoming release. 😄

@ghost
Copy link
Author

ghost commented Oct 8, 2020

I'm not sure if adding an accelerator not specified in the original code works.
I tried to test it, but none of the accelerators work on my Mac(even English env). I'll test it on a Windows PC.

I've tested this on Ubuntu 18.04.5 and there it works fine. It looks like I can add any accelerator I choose, even if no accelerator is defined in the original text yet.

Could you please give me more information on the issue?

Okay, so about

Links in tooltips, such as Regular Expression in Wikibooks, don't appear to work

See this video on Imgur. When I click help on the regular expression option (in the Search and Replace dialog), a link to Wikibooks appears, but when I click on it, nothing happens (at least on Ubuntu 18.04.5). Links in the About dialog, however, do work.

...and about

Tooltips with function documentation don't show Unicode (accented) characters,
they appear as the infamous replacement character

See this image on Imgur. When I type a SQLite function name in the SQL editor, documentation about the function appears in the tooltip. However, Unicode characters (characters with accents, like é, ë, ó, ï, etc.) are not rendered correctly, but shown as the replacement character �.

@mgrojo
Copy link
Member

mgrojo commented Oct 8, 2020

Links in tooltips, such as Regular Expression in Wikibooks, don't appear to work

See this video on Imgur. When I click help on the regular expression option (in the Search and Replace dialog), a link to Wikibooks appears, but when I click on it, nothing happens (at least on Ubuntu 18.04.5). Links in the About dialog, however, do work.

I added that link and later realized that it cannot be clicked. I left it, nevertheless, thinking that it might be some glitch in my environment. We can remove automatically from all the translations if we confirm that it is broken and has no possible fix.

...and about

Tooltips with function documentation don't show Unicode (accented) characters,
they appear as the infamous replacement character

See this image on Imgur. When I type a SQLite function name in the SQL editor, documentation about the function appears in the tooltip. However, Unicode characters (characters with accents, like é, ë, ó, ï, etc.) are not rendered correctly, but shown as the replacement character �.

That's strange, because it seems like a reproduction of #1206, one of the first bugs that I supposedly fixed in sqlitebrowser 😃 I don't reproduce it in Spanish, and now in Windows and v3.12.0. Which encoding are you using in your platform?

@ghost
Copy link
Author

ghost commented Oct 8, 2020

@mgrojo My system uses nl_NL.UTF-8 and I'm simply using the standard Qt Linguist software to edit the .ts file, which
I assume simply edits and saves in UTF-8, as well.

To be sure perhaps you could try to run the .ts file (or only the relevant bits with the accented chars) through some encoding detection software to verify whether it's actually UTF-8, or I could try to do this tomorrow for you, as well.

However, having quickly reviewed the commit file in your mentioned bug report, I see that it calls the method isUtf8() and then mutates the QByteArray to either Latin 1 or UTF 8. It might be saver to simply always use UTF-8 since (I believe, not entirely sure) Latin 1 is a subset of UTF-8 anyway, unless there are some other intricacies with the isUtf8() method and Scintilla, that I am unaware of. Sorry, ignore that part, only the first 128 bytes (ASCII) are a subset. If my statement was true, the � would probably not have appeared. 😉 But this point still stands:

My suspicion is that isUtf8() incorrectly assesses the CODEPAGE of the environment (or of the language, or whatever it bases its decision on) as Latin 1, when it actually should assess it as UTF-8. But I know next to nothing about DB4S' codebase, Qt or Scintilla, so I'm just guessing based on the quick glance I had in your mentioned commit.

Hope this helps.

@mgrojo
Copy link
Member

mgrojo commented Oct 8, 2020

In your image the lag() function appears, but it is not currently defined in the translation files, so I'm a bit confused.
Edit: ok, I've found it now. Everything seems in order in the translation file.

@mgrojo
Copy link
Member

mgrojo commented Oct 9, 2020

@Codifier, I was writing that I don't reproduce the problem, that the call tip for lag(expr,offset) shows correctly the 'ë' in 'geëvalueerd' and diacritics in Spanish are also displayed correctly. But after playing a bit I've discovered that you have to select the lag() version using the arrows in order to reproduce the problem. If you access the tip writing lag(expr, then the text is displayed correctly. So it seems that there is still a problem in QScintilla. I will take a look.

@ghost
Copy link
Author

ghost commented Oct 9, 2020

@mgrojo O wow, you are right! I did not notice earlier that it does work correctly when it shows the direct function documentation, when typing the exact function signature.

I wish I could be of more help to you, but I'm afraid I'd probably have to take a course on Qt, Scintilla and probably C++, as well, first. 😬 But good luck — I'm sure you will find it.

And, to be fair, I don't think it's really a show stopper. It's a mild annoyance at most, in my opinion. Especially now that you've discovered it only happens when pressing the arrow.

mgrojo added a commit that referenced this pull request Oct 9, 2020
…cters

There is a further problem when the calltip is displayed after clicking
one of the arrows for overloaded functions.

This was first reported in #1107 (Russian) and #1206 (Korean). The subcase
has been reported in PR #2424.

Patch reported in the QScintilla list, so there is no need to keep a new
patch. It is assumed to come in new QScintilla versions.
mgrojo added a commit that referenced this pull request Oct 9, 2020
…cters

There is a further problem when the calltip is displayed after clicking
one of the arrows for overloaded functions.

This was first reported in #1107 (Russian) and #1206 (Korean). The subcase
has been reported in PR #2424.

Patch reported in the QScintilla list, so there is no need to keep a new
patch. It is assumed to come in new QScintilla versions.
@mgrojo
Copy link
Member

mgrojo commented Oct 9, 2020

Don't worry. I was easy after the first correction. Now it's fixed in the master and v3.12.x branches. Thanks for pointing it out.

@ghost
Copy link
Author

ghost commented Oct 9, 2020

@mgrojo Nice one! 👍 And you're welcome.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants