Unify product name to as few places as possible #7192

luke-jr · 2015-12-09T11:50:29Z

This should help keep the software name consistent within translations (Bitcoin Kern, Nucleul Bitcoin, etc) or forks (Bitcoin LJR, etc).

btcdrak · 2015-12-09T11:51:56Z

Good.

utACK

maflcko · 2015-12-09T12:04:54Z

src/init.cpp

As you are touching this translation anyway. Is there any language that translates 2009 into something other than 2009? If so, this should be changed as well.

Agree, makes sense to parametrize both years, even though the first one will never change

jonasschnelli · 2015-12-09T12:53:19Z

Nice!
Concept ACK.
Will test soon.

jonasschnelli · 2015-12-10T07:45:47Z

Currently fails windows: https://bitcoin.jonasschnelli.ch/pulls/7192/build-win.log i'm happy to test this if windows issues are solved.

laanwj · 2015-12-10T11:46:14Z

src/qt/bitcoingui.cpp

This won't work. And you don't want the tr. Just use:

QString windowTitle = QString(PACKAGE_NAME) + " - ";

But then it won't be translated? I don't mean to change that status quo in this PR...

Oh, right, does this work then? Does qtranslate pick up the string out of tr(PACKAGE_NAME), even though it's a macro? Same for _(PACKAGE_NAME) in gettext? I thought only literal strings could be input to translation.

I doubt it. I imagine there's some way to explicitly tell it? :/

I believe this has been addressed here.

laanwj · 2015-12-10T11:47:10Z

Concept ACK (for 0.13 - due to translation changes this is too late for 0.12)

maflcko · 2015-12-10T17:51:43Z

src/qt/forms/debugwindow.ui

Needs rebase to at least 5940cf33b35fb70979a7baa3046107a9b72d8f0f

luke-jr · 2015-12-11T08:15:51Z

Added a second commit reworking the binary blobs used for Mac-deploy. Haven't rebased yet pending completion/testing.

@laanwj I am probably going to need to do the translation updates for Bitcoin LJR anyway, so let me know and I can see about doing it in Transifex for 0.12 if that'd be preferable.

pstratem · 2015-12-11T10:05:16Z

concept ACK 0898436e860f695028b3fc6b612074712bbdcbc6
(reviewed the code also, but don't know anything about qt/autotools)

luke-jr · 2015-12-13T05:59:46Z

Ok, I believe I have addressed all previous requests, but also introduced a packaging FIXME which I hope @theuni can help out with...

laanwj · 2016-01-22T15:22:51Z

This gives me an empty translation string in bitcoinstrings.cpp, the output of GNU gettext:

+QT_TRANSLATE_NOOP("bitcoin-core", ""),

I'm not sure where it comes from - I can't find any direct _("") in the code. But it indicates something is wrong, maybe a _() that gets passed a macro or variable.

*Note: * after merging this, need to re-run autogen.sh and configure otherwise there will be an undefined macro error during compile.

laanwj · 2016-01-25T14:36:24Z

@luke-jr It had nothing to do with what I thought above. You can apply the following diff to prevent adding an empty translation string:

diff --git a/share/qt/extract_strings_qt.py b/share/qt/extract_strings_qt.py
index 2a6e4b9..cd76fab 100755
--- a/share/qt/extract_strings_qt.py
+++ b/share/qt/extract_strings_qt.py
@@ -72,7 +72,7 @@ f.write("""
 f.write('static const char UNUSED *bitcoin_strings[] = {\n')
 f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('PACKAGE_NAME'),))
 f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('COPYRIGHT_HOLDERS'),))
-if os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') != os.getenv('PACKAGE_NAME'):
+if os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') and os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION') != os.getenv('PACKAGE_NAME'):
     f.write('QT_TRANSLATE_NOOP("bitcoin-core", "%s"),\n' % (os.getenv('COPYRIGHT_HOLDERS_SUBSTITUTION'),))
 messages.sort(key=operator.itemgetter(0))
 for (msgid, msgstr) in messages:

(probably makes sense to do this for the other getenvs as well, thinking of it)

luke-jr · 2016-01-25T17:20:18Z

Why would those variables ever be undefined or empty? :/

laanwj · 2016-01-27T09:34:58Z

As said, COPYRIGHT_HOLDERS_SUBSTITUTION ends up empty in my tests, resulting in an empty translation string. If this is not the intention something else must be going wrong.

…ons so it gets passed to extract-strings correctly

luke-jr · 2016-01-28T04:58:54Z

@laanwj Ok, fixed that.

Also fixed the text sizing logic in splashscreen to be more flexible with unexpected fonts and/or name lengths; and moved PACKAGE_URL from setup.nsi.in to configure.ac's AC_INIT (note that it uses the old bitcoin.org URL in the commit itself, and updated in the merge with master).

laanwj · 2016-01-28T09:49:00Z

configure.ac

Won't this make it much to easy to 'steal' copyright, by just changing this value?
We discussed something about only making it possible to add copyright holders this way, not replace the current ones.
Or am I misunderstanding how this works and this is true already?

It was always easy to 'steal'. This simply makes it easy to extend without stealing: changing the package name itself (in AC_INIT) won't automatically change the substitution here (as with the original PR), and it can be easily amended to "The %s and FooCoin developers".

Yes, it's easy to do anyway, if you really want to, but it should be as hard as possible to do so accidentally.
E.g. I strongly feel it should at least involve changing the source code, not just the build metadata.
Adding potential copyright holders in the build metadata is fine with me, although those in turn may have the same concern.

How about moving _COPYRIGHT_HOLDERS_SUBSTITUTION out of the top, down to the AC_SUBST area below?

This suggestion may sound stupid...but how about repeating the line if %s != "Bitcoin Core" (or just always repeating the line if that's too complicated)?
That way, when someone uses the code and wants to add something to the copyright, "The Bitcoin Core Developers" will be maintained first, before the modified one, which is what forks of this project should always do.

Moving it doesn't solve my issue; to be clear: having the copyright in a autoconf variable means it is passed to the compiler as a -D... flag. I think this is ridiculous. Copyright should be in the code, not something that can be varied by changing a compiler argument.

@jtimon's solution sounds somewhat better to me. It needs to be impossible to get rid of our copyright by just changing build metadata, without changing the actual code.

The problem with simply moving this to the code, is that the copyright notice appears outside of code also (typically in single-line format).

OK, fair enough. In this case I only care about the copyright instance in the code (which will be shown with --version and in the about box) that should be resilient to change of compiler arguments.

If there are other ones that are also in metadata, having them be modifiable in other metadata doesn't bother me.

Ok, how's this look now?

Looks good to me now, going to test

…ctly

…add a second line to copyrights in -version, About dialog, and splash screen

laanwj · 2016-02-04T12:36:41Z

Bitcoin Core Daemon version v0.12.99.0-b204181-dirty
Copyright (C) 2009-2016 The Shitcoin Core developers
Copyright (C) 2009-2016 The Bitcoin Core developers

(could nit on the years, probably derived software will have a different year range for their own development, but it's good enough for a fallback)
ACK a68bb9f

027fdb8 When/if the copyright line does not mention Bitcoin Core developers, add a second line to copyrights in -version, About dialog, and splash screen (Luke Dashjr) cc2095e Rewrite FormatParagraph to handle newlines within input strings correctly (Luke Dashjr) cddffaf Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substitutions so it gets passed to extract-strings correctly (Luke Dashjr) 29598e4 Move PACKAGE_URL to configure.ac (Luke Dashjr) 78ec83d splashscreen: Resize text to fit exactly (Luke Dashjr) 3cae140 Bugfix: Actually use _COPYRIGHT_HOLDERS_SUBSTITUTION everywhere (Luke Dashjr) 4d5a3df Bugfix: gitian-descriptors: Add missing python-setuptools requirement for OS X (biplist module) (Luke Dashjr) e4ab5e5 Bugfix: Correct copyright year in Mac DMG background image (Luke Dashjr) 917b1d0 Set copyright holders displayed in notices separately from the package name (Luke Dashjr) c39a6ff Travis & gitian-osx: Use depends for ds_store and mac_alias modules (Luke Dashjr) 902ccde depends: Add mac_alias to depends (Luke Dashjr) 82a2d98 depends: Add ds_store to depends (Cory Fields) de619a3 depends: Pass PYTHONPATH along to configure (Cory Fields) e611b6e macdeploy: Use rsvg-convert rather than cairosvg (Luke Dashjr) 63bcdc5 More complicated package name substitution for Mac deployment (Luke Dashjr) 1a6c67c Parameterise 2009 in translatable copyright strings (Luke Dashjr) d5f4683 Unify package name to as few places as possible without major changes (Luke Dashjr)

luke-jr force-pushed the single_prodname branch from e7bcf86 to 4f1b52b Compare December 9, 2015 11:53

maflcko reviewed Dec 9, 2015
View reviewed changes

luke-jr force-pushed the single_prodname branch from 4f1b52b to 81748c9 Compare December 10, 2015 06:47

luke-jr force-pushed the single_prodname branch 2 times, most recently from 364d118 to c1be808 Compare December 10, 2015 08:23

jonasschnelli added the Refactoring label Dec 10, 2015

luke-jr force-pushed the single_prodname branch from c1be808 to 648e964 Compare December 10, 2015 09:24

laanwj reviewed Dec 10, 2015
View reviewed changes

maflcko reviewed Dec 10, 2015
View reviewed changes

src/qt/forms/debugwindow.ui Outdated

Copy link

Member

maflcko Dec 10, 2015

Choose a reason for hiding this comment

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

Needs rebase to at least 5940cf33b35fb70979a7baa3046107a9b72d8f0f

luke-jr force-pushed the single_prodname branch from 648e964 to c903ae4 Compare December 11, 2015 08:12

luke-jr force-pushed the single_prodname branch from c903ae4 to 0898436 Compare December 11, 2015 09:07

luke-jr force-pushed the single_prodname branch 2 times, most recently from 0c15a7a to da198c7 Compare December 13, 2015 05:58

luke-jr force-pushed the single_prodname branch 7 times, most recently from fbca801 to 809d521 Compare December 14, 2015 01:40

Unify package name to as few places as possible without major changes

d5f4683

luke-jr force-pushed the single_prodname branch from 809d521 to a3481e9 Compare December 14, 2015 02:11

luke-jr added 3 commits January 27, 2016 20:29

splashscreen: Resize text to fit exactly

78ec83d

Move PACKAGE_URL to configure.ac

29598e4

Bugfix: Include COPYRIGHT_HOLDERS_SUBSTITUTION in Makefile substituti…

cddffaf

…ons so it gets passed to extract-strings correctly

luke-jr force-pushed the single_prodname branch from 601bdfc to ccca8b4 Compare January 28, 2016 04:56

laanwj reviewed Jan 28, 2016
View reviewed changes

luke-jr added 3 commits February 3, 2016 05:38

Rewrite FormatParagraph to handle newlines within input strings corre…

cc2095e

…ctly

When/if the copyright line does not mention Bitcoin Core developers, …

027fdb8

…add a second line to copyrights in -version, About dialog, and splash screen

Merge branch 'master' into single_prodname

a68bb9f

luke-jr force-pushed the single_prodname branch from ccca8b4 to a68bb9f Compare February 3, 2016 05:43

laanwj merged commit a68bb9f into bitcoin:master Feb 4, 2016

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Jun 30, 2016

Merge bitcoin#7192 single_prodname-0.12.x

4deeadb

maflcko mentioned this pull request Oct 24, 2016

Untranslated strings in GUI #8900

Closed

jonasschnelli mentioned this pull request Dec 22, 2016

Contrib: Mac: Fix mac deploy so that the dmg background works #9405

Closed

JeremyRand mentioned this pull request Jul 9, 2021

Consider factoring out "bitcoins" string in AskPassphraseDialog bitcoin-core/gui#380

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Unify product name to as few places as possible #7192

Unify product name to as few places as possible #7192

Uh oh!

Conversation

luke-jr commented Dec 9, 2015

Uh oh!

btcdrak commented Dec 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Dec 9, 2015

Uh oh!

jonasschnelli commented Dec 10, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Dec 10, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Dec 11, 2015

Uh oh!

pstratem commented Dec 11, 2015

Uh oh!

luke-jr commented Dec 13, 2015

Uh oh!

laanwj commented Jan 22, 2016

Uh oh!

laanwj commented Jan 25, 2016

Uh oh!

luke-jr commented Jan 25, 2016

Uh oh!

laanwj commented Jan 27, 2016

Uh oh!

luke-jr commented Jan 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Feb 4, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants