[Merge] lp:~phablet-team/webbrowser-app/webbrowser-app-desktop-translation into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Tue Jun 17 10:31:45 UTC 2014
Looks good overall. I have a handful of minor comments, please see inline.
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2014-05-30 16:16:56 +0000
> +++ CMakeLists.txt 2014-06-16 11:02:20 +0000
> @@ -1,6 +1,9 @@
> cmake_minimum_required(VERSION 2.8.9)
> set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
>
> +find_program(INTLTOOL_MERGE intltool-merge)
> +find_program(INTLTOOL_EXTRACT intltool-extract)
> +
> # Standard install paths
> include(GNUInstallDirs)
>
> @@ -37,6 +40,8 @@
> set(CMAKE_INCLUDE_CURRENT_DIR ON)
> set(CMAKE_AUTOMOC ON)
>
> +set(DESKTOP_FILE webbrowser-app.desktop)
> +
> # uninstall target
> configure_file(
> "${CMAKE_CURRENT_SOURCE_DIR}/cmake_uninstall.cmake.in"
> @@ -52,8 +57,10 @@
>
> file(GLOB_RECURSE I18N_SRC_FILES
> RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
> - src/app/**.qml src/app/**.desktop.in)
> + src/app/**.qml)
> +list(APPEND I18N_SRC_FILES src/app/webbrowser/webbrowser-app.desktop.in.in)
Please use ${DESKTOP_FILE} here to substitute "webbrowser-app.desktop".
> list(SORT I18N_SRC_FILES)
> +message("Found ${I18N_SRC_FILES}")
Please remove this debug log.
>
> # for dh_translations to extract the domain
> # (regarding syntax consistency, see http://pad.lv/1181187)
>
> === modified file 'po/CMakeLists.txt'
> --- po/CMakeLists.txt 2013-06-04 08:07:51 +0000
> +++ po/CMakeLists.txt 2014-06-16 11:02:20 +0000
> @@ -7,7 +7,11 @@
> set(POT_FILE ${DOMAIN}.pot)
> file(GLOB PO_FILES *.po)
>
> -add_custom_target(${POT_FILE}
> +add_custom_target(${POT_FILE} ALL
> + COMMENT "Generating translation template"
> + COMMAND ${INTLTOOL_EXTRACT} --update --type=gettext/ini
> + --srcdir=${CMAKE_SOURCE_DIR} src/app/webbrowser/${DESKTOP_FILE}.in.in
> +
It seems the above comment generates po/src/app/webbrowser/webbrowser-app.desktop.in.in.h in the source tree as an intermediate file to pick up translations.
Can you please add it to .bzrignore?
> COMMAND ${GETTEXT_XGETTEXT_EXECUTABLE} -o ${POT_FILE}
> -D ${CMAKE_SOURCE_DIR}
> --from-code=UTF-8
>
> === modified file 'po/webbrowser-app.pot'
> --- po/webbrowser-app.pot 2014-05-27 09:37:45 +0000
> +++ po/webbrowser-app.pot 2014-06-16 11:02:20 +0000
> @@ -8,7 +8,7 @@
> msgstr ""
> "Project-Id-Version: webbrowser-app\n"
> "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2014-05-06 16:25+0100\n"
> +"POT-Creation-Date: 2014-06-16 12:54+0200\n"
> "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
> "Last-Translator: FULL NAME <EMAIL at ADDRESS>\n"
> "Language-Team: LANGUAGE <LL at li.org>\n"
> @@ -89,7 +89,7 @@
> msgid "Forward"
> msgstr ""
>
> -#: src/app/Chrome.qml:127 src/app/webbrowser/ActivityView.qml:36
> +#: src/app/Chrome.qml:126 src/app/webbrowser/ActivityView.qml:36
> msgid "Activity"
> msgstr ""
>
> @@ -246,63 +246,51 @@
> msgid "Currently viewing (%1)"
> msgstr ""
>
> -#: src/app/webbrowser/TabsList.qml:68
> +#: src/app/webbrowser/TabsList.qml:69
> msgid "+"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:84
> +#: src/app/webbrowser/TimelineView.qml:87
> msgid "Today"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:86
> +#: src/app/webbrowser/TimelineView.qml:89
> msgid "Yesterday"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:88
> +#: src/app/webbrowser/TimelineView.qml:91
> msgid "Last 7 Days"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:90
> +#: src/app/webbrowser/TimelineView.qml:93
> msgid "This Month"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:92
> +#: src/app/webbrowser/TimelineView.qml:95
> msgid "This Year"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:94
> +#: src/app/webbrowser/TimelineView.qml:97
> msgid "Older"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:277
> +#: src/app/webbrowser/TimelineView.qml:287
> msgid "(local files)"
> msgstr ""
>
> -#: src/app/webbrowser/TimelineView.qml:279
> +#: src/app/webbrowser/TimelineView.qml:289
> msgid "(other)"
> msgstr ""
>
> -#: src/app/webbrowser/webbrowser-app.desktop.in:3
> -msgid "Browser"
> -msgstr ""
> -
> -#: src/app/webbrowser/webbrowser-app.desktop.in:4
> -msgid "Web Browser"
> -msgstr ""
> -
> -#: src/app/webbrowser/webbrowser-app.desktop.in:5
> -msgid "Browse the World Wide Web"
> -msgstr ""
> -
> #. TRANSLATORS: %1 refers to the current page’s title
> #: src/app/webbrowser/webbrowser-app.qml:35
> -#: src/app/webcontainer/webapp-container.qml:47
> +#: src/app/webcontainer/webapp-container.qml:53
> #, qt-format
> msgid "%1 - Ubuntu Web Browser"
> msgstr ""
>
> #: src/app/webbrowser/webbrowser-app.qml:37
> -#: src/app/webcontainer/webapp-container.qml:49
> +#: src/app/webcontainer/webapp-container.qml:55
> msgid "Ubuntu Web Browser"
> msgstr ""
>
> @@ -322,6 +310,6 @@
> msgid "Select an account"
> msgstr ""
>
> -#: src/app/webcontainer/WebViewImplWebkit.qml:63
> +#: src/app/webcontainer/WebViewImplWebkit.qml:68
> msgid "This page wants to know your device’s location."
> msgstr ""
>
> === modified file 'src/app/webbrowser/CMakeLists.txt'
> --- src/app/webbrowser/CMakeLists.txt 2014-04-02 13:58:42 +0000
> +++ src/app/webbrowser/CMakeLists.txt 2014-06-16 11:02:20 +0000
> @@ -46,14 +46,11 @@
> DESTINATION ${CMAKE_INSTALL_DATADIR}/webbrowser-app/webbrowser
> FILES_MATCHING PATTERN *.png)
>
> -set(DESKTOP_FILE webbrowser-app.desktop)
> -configure_file(${DESKTOP_FILE}.in.in ${DESKTOP_FILE}.in @ONLY)
> -file(REMOVE ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE})
> -file(STRINGS ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE}.in DESKTOP_FILE_CONTENTS)
> -foreach(LINE ${DESKTOP_FILE_CONTENTS})
> - string(REGEX REPLACE "tr\\\(\"(.*)\"\\\)" "\\1" LINE "${LINE}")
> - file(APPEND ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE} "${LINE}\n")
> -endforeach(LINE)
> +configure_file(${DESKTOP_FILE}.in.in ${DESKTOP_FILE}.in)
Why did the @ONLY parameter disappear?
> +add_custom_target(${DESKTOP_FILE} ALL
> + COMMENT "Merging translations into ${DESKTOP_FILE}"
> + COMMAND LC_ALL=C ${INTLTOOL_MERGE} -d -u ${CMAKE_SOURCE_DIR}/po ${DESKTOP_FILE}.in ${DESKTOP_FILE} >/dev/null
Are "LC_ALL=C" and ">/dev/null" really needed?
I tried removing them, and the desktop file seemed to be generated just fine.
> + )
>
> install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${DESKTOP_FILE}
> DESTINATION ${CMAKE_INSTALL_DATADIR}/applications)
>
> === modified file 'src/app/webbrowser/webbrowser-app.desktop.in.in'
> --- src/app/webbrowser/webbrowser-app.desktop.in.in 2013-11-25 16:04:05 +0000
> +++ src/app/webbrowser/webbrowser-app.desktop.in.in 2014-06-16 11:02:20 +0000
> @@ -1,14 +1,14 @@
> [Desktop Entry]
> Version=1.0
> -Name=tr("Browser")
> -GenericName=tr("Web Browser")
> -Comment=tr("Browse the World Wide Web")
> +_Name=Browser
> +_GenericName=Web Browser
> +_Comment=Browse the World Wide Web
> +_Keywords=Internet;WWW;Browser;Web;Explorer
> Type=Application
> Icon=webbrowser-app
> Exec=webbrowser-app %u
> Terminal=false
> Categories=Network;WebBrowser;
> -Keywords=Internet;WWW;Browser;Web;Explorer
> MimeType=text/html;text/xml;application/xhtml+xml;x-scheme-handler/http;x-scheme-handler/https;
> X-Ubuntu-Touch=true
> X-Ubuntu-Gettext-Domain=webbrowser-app
>
--
https://code.launchpad.net/~phablet-team/webbrowser-app/webbrowser-app-desktop-translation/+merge/223219
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~phablet-team/webbrowser-app/webbrowser-app-desktop-translation into lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list