Enable Translations #89

achow101 · 2016-09-09T04:00:08Z

I got most (nearly all I think) of the GUI strings that will need to be translated wrapped in the tr() function. I'm sure I missed a few, but I may have accidentally wrapped a few things that shouldn't be translated, so be sure to check for that during review.

I've also added the resource files for translations, but no code yet to handle actual translations. These resource files are from Transifex (except English) but I don't think the translations have been reviewed, so they may not be accurate. However, these translations won't be in use until languages can be selected.

Also, any new strings added that will appear in the GUI will need to be wrapped in the tr() function. pylupdate4 will then need to be run to generate the English resource file and that file uploaded to Transifex.

goatpig · 2016-09-09T07:48:06Z

How does this work exactly? Do we have to implement a language selector in app?

achow101 · 2016-09-09T12:15:57Z

At some point, yes we will need to implement a language selector. I'd put it in the settings. There will also need to be a change in the build process to add lupdate so that the language stuff will work. I'm looking into how doing all of this translation stuff should be done.

Also, getting the language selector isn't really urgent yet since no languages are close to being fully translated yet (after I tripled the number of strings to be translated).

goatpig · 2016-09-09T12:18:25Z

You could replace all entries with a lorem ipsum just for the sake of testing though. I'm more concerned about breaking some far fetched 6 layer deep dialog than the actual quality of completion of tsl.

achow101 · 2016-09-09T12:21:14Z

well first we need to implement a language selector. RIght now, this breaks nothing. The only thing I am really concerned about are a few strings that had weird format stuff that I changed (not sure if I changed them correctly) and that I may have accidentally added something that wasn't a GUI string to be translated (e.g. an identifier for something in the wallet).

goatpig · 2016-09-09T12:23:02Z

At any rate the first step is reviewing and testing these. Last time one of the ATI devs tried to tr() everything in the Armory code base, it blew up in epic proportion and 99% of it was rolled back.

achow101 · 2016-09-09T12:25:47Z

Right... There are actually a few strings that probably should have been left alone due to the weird formatting thing that was happening with them. I'll see if I can find them (qtdialogs is just massive) and revert them.

goatpig · 2016-09-09T12:39:33Z

Regardless this is a good time to do this change, right at the end of the early testing phase.

achow101 · 2016-09-09T15:21:46Z

armorymodels.py

@@ -1114,7 +1114,7 @@ def data(self, index, role=Qt.DisplayRole):
            return QVariant( cppAddr.getTxioCount())
         if col==COL.ChainIdx:
            if self.wlt.addrMap[addr160].chainIndex==-2:
-               return QVariant('Imported')
+               return QVariant(tr('Imported'))


Is this a label that is displayed or something for the wallet internally?

achow101 · 2016-09-09T15:34:50Z

The string at https://github.com/achow101/BitcoinArmory/blob/355db40ad408008a407587de92838afe1529b493/qtdialogs.py#L4210 is questionable as to whether I formatted it correctly.

droark · 2016-09-09T16:13:33Z

utACK

No changes jumped out at me. Looks good. I feel like there was a reason Alan stopped using tr(), though. I can't remember why. I know there was an attempt at adding translations but it was pulled eventually. Anyone remember?

In any event, other than having to remember to wrap any new text in tr(), I'm fine with this commit. I'll test on OSX once it's finally running again.

josephbisch · 2016-09-09T17:46:01Z

utACK

Nothing jumped out at me either. However, I just scrolled through a lot of it, but not all.

The questionable string (L4210 of qtdialogs.py) looks good to me, but I think the %s format specifiers will literally show up in the string to be translated, if I understand how this works. So the translators will need to know to leave the %s characters as is when they translate the string. The only other issue I see is that the use of %s to format the string leaves less flexibility for the translators. For example, what if theoretically there is a language that makes words plural in some way other than appending a character on the end of the word. But that is probably overthinking this and I don't know how you would handle that with PyQt.

achow101 · 2016-09-09T21:07:59Z

I've replaced the tr()s with self.tr()s. I also squashed the commits.

achow101 · 2016-09-10T02:05:41Z

It turns out tr() does work with the % formatting that Armory mostly uses for strings. I am now slowly going through the strings again and replacing them with QString's .arg() function.

achow101 · 2016-09-10T04:04:20Z

Ok, I think I fixed everything and it's ready for review, and merging. I did some testing.

You can use achow101@c5b56e0 for testing. Just change the name of the .qm file to whatever other language you want to try translating. You will have to run lrelease lang/*.ts first.

achow101 · 2016-09-10T16:13:52Z

I've finished everything that needs to be done for translations. You can now change the language in the settings.

All that's left is to wait for all of the translations to be completed. And testing and review of all of the changes.

Now if someone could make a test translation with a bunch of lorem ipsums....

goatpig · 2017-02-13T15:07:03Z

ArmoryQt.py

+qt4reactor.install()
+
+# Setup translations
+langSetting = SettingsFile(CLI_OPTIONS.settingsPath).get('Language')


This should be in ArmoryUtils, where it can also be detected a cli arg if needs be.

goatpig · 2017-02-13T15:10:40Z

ArmoryQt.py

                  No passphrase was selected for the encrypted backup.
                  No backup was created"""), QMessageBox.Ok)
-            newPassphrase = SecureBinaryData(str(dlgCrypt.edtPasswd1.text()))
+            newPassphrase = SecureBinaryData(unicode(dlgCrypt.edtPasswd1.text()))


This looks like a wild search and replace. This is not actual text, it is casting a QString to a python string for the SWIG constructor. These casts are necessary for SWIG to work.

goatpig · 2017-02-13T15:14:39Z

ArmoryQt.py

-      currComment = str(view.model().index(row, LEDGERCOLS.Comment).data().toString())
-      wltID       = str(view.model().index(row, LEDGERCOLS.WltID  ).data().toString())
-      txHash      = str(view.model().index(row, LEDGERCOLS.TxHash ).data().toString())
+      currComment = unicode(view.model().index(row, LEDGERCOLS.Comment).data().toString())


This is a dangerous cast. Legacy Armory wallets cannot support multi byte characters. This cast would allow to convert actual unicode characters to multi byte values that would then be written to the wallet, breaking the container.

goatpig · 2017-02-13T15:16:08Z

ArmoryQt.py

-      txHash      = str(view.model().index(row, LEDGERCOLS.TxHash ).data().toString())
+      currComment = unicode(view.model().index(row, LEDGERCOLS.Comment).data().toString())
+      wltID       = unicode(view.model().index(row, LEDGERCOLS.WltID  ).data().toString())
+      txHash      = unicode(view.model().index(row, LEDGERCOLS.TxHash ).data().toString())


This would break comparison functions. Hashes in wallets are strings, and comments are keyed by strings. It would fail to find the entry later if it is keyed as a unicode object.

goatpig · 2017-02-13T15:17:00Z

ArmoryQt.py

-      currComment = str(view.model().index(row, ADDRESSCOLS.Comment).data().toString())
-      addrStr     = str(view.model().index(row, ADDRESSCOLS.Address).data().toString())
+      currComment = unicode(view.model().index(row, ADDRESSCOLS.Comment).data().toString())
+      addrStr     = unicode(view.model().index(row, ADDRESSCOLS.Address).data().toString())


Same comment as above with comment and hashes. These need to be str, not unicode

goatpig · 2017-02-13T16:15:44Z

qtdialogs.py

@@ -12410,10 +12395,10 @@ def verifyUserInput(self):
      if self.chkEncrypt.isChecked():
         dlgPasswd = DlgChangePassphrase(self, self.main)
         if dlgPasswd.exec_():
-            passwd = SecureBinaryData(str(dlgPasswd.edtPasswd1.text()))
+            passwd = SecureBinaryData(unicode(dlgPasswd.edtPasswd1.text()))


goatpig · 2017-02-13T16:16:47Z

qtdialogs.py

@@ -13200,9 +13184,9 @@ def processFrags(self):
      if self.chkEncrypt.isChecked():
         dlgPasswd = DlgChangePassphrase(self, self.main)
         if dlgPasswd.exec_():
-            passwd = SecureBinaryData(str(dlgPasswd.edtPasswd1.text()))
+            passwd = SecureBinaryData(unicode(dlgPasswd.edtPasswd1.text()))


goatpig · 2017-02-13T16:17:26Z

qtdialogs.py

               The ID field indicates that this is a SecurePrint™
               Backup Type. You have either entered the ID incorrectly or
               have chosen an incorrect Backup Type."""), QMessageBox.Ok)
            return
      for i in rng:
         hasError = False
         try:
-            rawEntry = str(self.edtList[i].text())
+            rawEntry = unicode(self.edtList[i].text())


readSixteenBytesEasy input, should be str

goatpig · 2017-02-13T16:26:07Z

ui/WalletFrames.py


   def getDescription(self):
-      return str(self.editDescription.toPlainText())
+      return unicode(self.editDescription.toPlainText())


These return values are probably saved in the lockbox container, should be str to be on the safe side.

goatpig · 2017-02-13T16:27:12Z

ui/WalletFrames.py

         if self.passphraseCallback:
            self.passphraseCallback()
      return result

   def getPassphrase(self):
-      return str(self.editPasswd1.text())
+      return unicode(self.editPasswd1.text())


Passphrase will probably get SWIG casted right away, this should be str.

achow101 · 2017-02-13T19:31:38Z

I have addressed your comments (the unicode stuff in particular) and squashed and rebased.

achow101 · 2017-02-13T22:08:45Z

Now I remember why those were put in unicode. Many languages don't have ASCII and some of the things that were put into unicode were because that part of the code would be reading things being displayed for some reason, but those were in other languages that didn't use ASCII and thus caused crashes.

goatpig · 2017-02-14T11:46:11Z

ArmoryQt.py

@@ -3242,7 +3262,7 @@ def showLedgerTx(self):
      row = self.ledgerView.selectedIndexes()[0].row()
      txHash = str(self.ledgerView.model().index(row, LEDGERCOLS.TxHash).data().toString())
      wltID  = str(self.ledgerView.model().index(row, LEDGERCOLS.WltID).data().toString())
-      txtime = unicode(self.ledgerView.model().index(row, LEDGERCOLS.DateStr).data().toString())
+      txtime = unstricode(self.ledgerView.model().index(row, LEDGERCOLS.DateStr).data().toString())


goatpig · 2017-02-14T11:50:22Z

armorymodels.py

@@ -251,50 +251,50 @@ def data(self, index, role=Qt.DisplayRole):
            isCB  = rowData[COL.isCoinbase]
            isConfirmed = (nConf>119 if isCB else nConf>5)
            if isConfirmed:
-               return QVariant('Transaction confirmed!\n(%d confirmations)'%nConf)
+               return QVariant(self.tr('Transaction confirmed!\n(%d confirmations)')%nConf)


Is this a proper way to feed in % args? You were using self.tr().arg() elsewhere.

goatpig · 2017-02-14T11:55:41Z

ui/MultiSigDialogs.py

@@ -977,11 +975,11 @@ def createLockboxDashboardTab(self):
      # simulfunding widgets
      self.stkDashboard = QStackedWidget()

-      simultxt = 'Simul'
+      simultxt = self.tr('SimulFund')


Do we really want translators to figure out translations for this? This is an ATI trademark too afaik, better off leaving it as is.

It came up in a transifex issue where the translator was saying that some of the strings say "check the Simul box" (or something like that) but Simul was not in the strings to be translated. If SimulFund is trademarked, it shouldn't be translated. Should I leave that untranslated for translate Simul

It should be left as SimulFund then, without a tr encapsulation. Sadly I believe translating a trademark without the owner's approval is a form of trademark law violation.

goatpig · 2017-02-14T12:00:00Z

ui/TxFrames.py

@@ -1317,12 +1314,12 @@ def __init__(self, parent=None, main=None, initLabel=''):
      self.sentToSelfWarn = False
      self.fileLoaded = None

-      lblDescr = QRichLabel(\
+      lblDescr = QRichLabel(tr(


missing self.

goatpig · 2017-02-14T12:00:47Z

ui/TxFrames.py

      self.infoLbls[-1].append(QRichLabel(''))

      # ##
      self.infoLbls.append([])
-      self.infoLbls[-1].append(self.main.createToolTipWidget(\
+      self.infoLbls[-1].append(self.main.createToolTipWidget(tr(


missing self.

goatpig · 2017-02-14T12:02:35Z

ui/TxFrames.py

-         QMessageBox.warning(self, 'No Internet!', \
-            'Armory lost its connection to Bitcoin-Core, and cannot '
+         QMessageBox.warning(self, self.tr('No Internet!'), \
+            self.tr('Armory lost its connection to , and cannot '


Bitcoin-Core removed as a whole in the few following sentences. No rephrasing though.

goatpig · 2017-02-14T12:58:15Z

Updated the code review

achow101 · 2017-02-14T14:47:25Z

Updated with fixes.

Wrapped all strings that need to be translated with self.tr for Qt translation. Replaces the % formatters with %1, %2 to use with QString arg() function instead. Fixes several typos that were found during translations Added the language files downloaded from Transifex. The English file is the source generated with pylupdate4

Added a language selector and CLI arg to change the language for the translations stuff

goatpig · 2017-02-14T23:53:18Z

qtdialogs.py

         nextAddr = wlt.peekNextUnusedAddr()

         addrStr = self.getAddrStr(wlt, nextAddr)
-         self.lblSelectWlt.setText('Will create new address: %s...' % \


Why was this removed?

probably a bad rebase/squash. I have added it back in.

goatpig · 2017-02-14T23:56:37Z

qtdialogs.py

+         f.write(self.tr('Export Date: %1\n').arg(unixTimeToFormatStr(RightNow())))
+         f.write(self.tr('Total Funds: %1\n').argcoin2str(totalFunds, maxZeros=0).strip())
+         f.write(self.tr('Spendable Funds: %1\n').argcoin2str(spendFunds, maxZeros=0).strip())
+         f.write(self.tr('Unconfirmed Funds: %1\n').argcoin2str(unconfFunds, maxZeros=0).strip())


These 3 lines missing one set of parenthesis around coin2str()

goatpig · 2017-02-15T00:16:16Z

ArmoryQt.py

-            'now?', QMessageBox.Yes | QMessageBox.No)
+         reply = QMessageBox.information(self, self.tr('No Wallets!'), self.tr('''
+            You just clicked on a "bitcoin:" link to send money, but you 
+            urrently have no wallets!  Would you like to create a wallet 


Fixes for plural forms. Typo fixes. The dashboard labels requires stripping strings, but str() does not support some characters used in other languages, so they need to be moved to unicode()

achow101 · 2017-02-15T18:18:06Z

This has been merged as commits 03cef63...f3353eb

goatpig closed this Sep 9, 2016

goatpig reopened this Sep 9, 2016

achow101 reviewed Sep 9, 2016
View reviewed changes

achow101 force-pushed the translations-setup branch from ed0aae5 to ba33108 Compare September 9, 2016 20:41

achow101 force-pushed the translations-setup branch 3 times, most recently from cd7b139 to f6cc697 Compare September 10, 2016 00:06

achow101 force-pushed the translations-setup branch 3 times, most recently from ffe8f1f to a37a506 Compare September 10, 2016 04:01

achow101 changed the title ~~Translations setup~~ Enable Translations Sep 10, 2016

achow101 force-pushed the translations-setup branch 5 times, most recently from 75f6ab2 to 8c813d9 Compare September 16, 2016 20:51

achow101 force-pushed the translations-setup branch from 35ee948 to 3332100 Compare February 10, 2017 14:28

goatpig requested changes Feb 13, 2017

View reviewed changes

achow101 force-pushed the translations-setup branch 3 times, most recently from d83429d to 64e9fa2 Compare February 13, 2017 19:31

achow101 force-pushed the translations-setup branch from 373d932 to 56a2ff6 Compare February 13, 2017 22:22

goatpig requested changes Feb 14, 2017

View reviewed changes

achow101 force-pushed the translations-setup branch from 56a2ff6 to da1e7b1 Compare February 14, 2017 14:46

achow101 force-pushed the translations-setup branch 3 times, most recently from 8409194 to 2fc0015 Compare February 14, 2017 15:42

achow101 mentioned this pull request Feb 14, 2017

Typos in some user-facing strings corrected #88

Closed

achow101 added 2 commits February 14, 2017 12:56

Language selector and CLI arg

74d961b

Added a language selector and CLI arg to change the language for the translations stuff

achow101 force-pushed the translations-setup branch 4 times, most recently from d7c5c81 to ab2b5a0 Compare February 14, 2017 20:43

goatpig reviewed Feb 15, 2017

View reviewed changes

Fix plurals; additional string fixes; Unicode in dashboard labels

7719e93

Fixes for plural forms. Typo fixes. The dashboard labels requires stripping strings, but str() does not support some characters used in other languages, so they need to be moved to unicode()

achow101 force-pushed the translations-setup branch from ab2b5a0 to 7719e93 Compare February 15, 2017 00:43

achow101 closed this Feb 15, 2017

This was referenced Feb 15, 2017

Plural strings #76

Closed

Various string issues #77

Closed

achow101 deleted the translations-setup branch March 3, 2017 20:29

Enable Translations #89

Enable Translations #89

Conversation

achow101 commented Sep 9, 2016

goatpig commented Sep 9, 2016

achow101 commented Sep 9, 2016

goatpig commented Sep 9, 2016

achow101 commented Sep 9, 2016

goatpig commented Sep 9, 2016 • edited

achow101 commented Sep 9, 2016

goatpig commented Sep 9, 2016

Choose a reason for hiding this comment

achow101 commented Sep 9, 2016

droark commented Sep 9, 2016

josephbisch commented Sep 9, 2016

achow101 commented Sep 9, 2016

achow101 commented Sep 10, 2016

achow101 commented Sep 10, 2016

achow101 commented Sep 10, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 commented Feb 13, 2017

achow101 commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 Feb 14, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goatpig Feb 14, 2017 • edited

Choose a reason for hiding this comment

goatpig commented Feb 14, 2017

achow101 commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 commented Feb 15, 2017

goatpig commented Sep 9, 2016 •

edited

achow101 commented Sep 10, 2016 •

edited

achow101 Feb 14, 2017 •

edited

goatpig Feb 14, 2017 •

edited