Skip to content

Read translations from .vue files too - #8094

Closed
nickvergessen wants to merge 1 commit into
masterfrom
bugfix/noid/read-translations-for-vue-files-too
Closed

Read translations from .vue files too#8094
nickvergessen wants to merge 1 commit into
masterfrom
bugfix/noid/read-translations-for-vue-files-too

Conversation

@nickvergessen

@nickvergessennickvergessen commented Jan 29, 2018

Copy link
Copy Markdown
Member

Don't know how to run the script local. @MorrisJobke to the rescue

@nickvergessennickvergessen added bug 3. to review Waiting for reviews feature: language/translations (l10n/i18n) Localization and translation matters labels Jan 29, 2018
@nickvergessennickvergessen added this to the Nextcloud 14 milestone Jan 29, 2018
@nickvergessen

nickvergessen commented Jan 29, 2018

Copy link
Copy Markdown
MemberAuthor

For testing see the empty translation list of the notifications app.

@codecov

codecovBot commented Jan 29, 2018•
edited
Loading

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@01482b3). Click here to learn what that means.
The diff coverage is n/a.

@@ Coverage Diff @@## master #8094 +/- ##
========================================
Coverage ? 51.7% Complexity ? 25426 ========================================
Files ? 1598 Lines ? 95236 Branches ? 1376 ========================================
Hits ? 49244 Misses ? 45992 Partials ? 0

@nickvergessen
Read translations from .vue files too
d1618f1
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/read-translations-for-vue-files-too branch from d1d45e4 to d1618f1CompareJanuary 29, 2018 14:25
@nickvergessen

nickvergessen commented Jan 29, 2018

Copy link
Copy Markdown
MemberAuthor

Okay, got it working now and confirmed its working.

For extra apps nextcloud/docker-ci#73 is necessary

nickvergessen
nickvergessen commented Jan 29, 2018
View reviewed changes
Comment threadl10n/l10n.pl
foreach my $i ( @files ){
next if substr( $i, 0, 1 ) eq '.';
next if $i eq 'l10n';
next if $i eq 'node_modules';

@nickvergessennickvergessenJan 29, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I also made it skip node_modules by default...

@nickvergessen

nickvergessen commented Jan 30, 2018•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

I noticed, that the way I use currently is quite unsatisfying for apps with a lot of strings in the UI:
https://github.com/nextcloud/notifications/blob/b59df847cf8a6fcc906e97496784d8446f8371f1/js-src/components/root.vue#L38-L40

Maybe it's time to write a component for t() and n(), or how did you experience this stuff?
@ChristophWurst@juliushaertl
nextcloud/notifications#114

@nickvergessen

nickvergessen commented Feb 2, 2018

Copy link
Copy Markdown
MemberAuthor

Removed by #6265 -> closing

@nickvergessen
nickvergessen deleted the bugfix/noid/read-translations-for-vue-files-too branch February 2, 2018 11:18
@MorrisJobkeMorrisJobke removed this from the Nextcloud 14 milestone Feb 2, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobkeMorrisJobkeAwaiting requested review from MorrisJobke
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsbugfeature: language/translations (l10n/i18n)Localization and translation matters

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nickvergessen@MorrisJobke