-
Notifications
You must be signed in to change notification settings - Fork 272
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
Multiterm: make strings translatable #254
Conversation
@@ -24,6 +24,8 @@ multiterm_la_SOURCES = \ | |||
terminal.vala | |||
|
|||
multiterm_la_CFLAGS = \ | |||
-DGETTEXT_PACKAGE='"$(GETTEXT_PACKAGE)"' \ | |||
-DLOCALEDIR='"$(localedir)"' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively you could probably have a config.h VAPI module, something like this:
[CCode(cheader="config.h", lower_case_cprefix="")]
namespace Config {
const string GETTEXT_PACKAGE;
const string LOCALEDIR;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I like that way better, and IIRC I've done it before on some project. If you feel like doing it go ahead, otherwise this way is fine too, at least the globals are constant.
Would it make more sense to have these in the Geany .vapi?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you feel like doing it go ahead, otherwise this way is fine too, at least the globals are constant.
I won't do it today, but you can ;)
Would it make more sense to have these in the Geany .vapi?
I don't think so no, because Geany doesn't provide it, nor depend on it. Well, GETTEXT_PACKAGE
a little as if it's not defined we will alias _
to a no-op, but that's about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, actually I just tried and it's not possible for LOCALEDIR
as it has to be changeable when running Make itself, so it has to be passed through there.
So we could have this for GETTEXT_PACKAGE
, but I doubt it's very sensible to use it only for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The again, I'm a tiny be stupid as LOCALEDIR
is already passed through AM_CFLAGS
from vars.build.mk, so it's redundant here.
Looks good to me, though I haven't had a chance to test Autotools compiling with the changes yet. |
Herm, makes me realize I didn't test the Waf build system, and it's likely not to work. |
f02169c
to
c73bacf
Compare
Waf is fixed |
c73bacf
to
467b007
Compare
Multiterm: make strings translatable
Closes #247.
I'm not quite sure whether https://github.com/geany/geany-plugins/blob/master/multiterm/src/shell-config.vala#L48 should be translated or not, but I left it alone.
Also, being able to translate the default profile name would be nice. This would be quite easy if this was a real file that would get installed instead of a string template. Or the template could be built with a translation in the middle.