Skip to content
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

Trim HTML content #68

Merged
merged 1 commit into from
Dec 10, 2019
Merged

Trim HTML content #68

merged 1 commit into from
Dec 10, 2019

Conversation

Chocobozzz
Copy link
Contributor

@Chocobozzz Chocobozzz commented Oct 23, 2019

Hi,

This is a proposal. Before fixing tests and adding this feature under a flag (because it could break some existing setups) I wanted to know if you were interested.

In HTML we can add extra spaces and line breaks (when we have long lines for example) with no effects. But easygettext extracts these extra spaces/line breaks which can confuse translators.

Example in Weblate:

screen_2019-10-23-15:41:54

This PR removes them using the https://medium.com/@patrickbrosset/when-does-white-space-matter-in-html-b90e8a7cdd33 algorithm

So:

#: src/views/FAQ.vue:115
msgid ""
"<strong>It's software you install on your server</strong> to create a website where videos are hosted and broadcast...\n"
"            Basically: you create your own \"homemade YouTube\"!"

becomes

msgid "<strong>It's software you install on your server</strong> to create a website where videos are hosted and broadcast... Basically: you create your own \"homemade YouTube\"!"

And the image above becomes in weblate:

screen_2019-10-23-15:47:43

@Chocobozzz Chocobozzz changed the title Trip HTML content Trim HTML content Oct 24, 2019
@vperron
Copy link
Contributor

vperron commented Oct 30, 2019

It's indeed extremely interesting and to be put under feature flag as you mentioned it :)

@vperron
Copy link
Contributor

vperron commented Dec 6, 2019

Hi ! Any news about this @Chocobozzz ?

@Chocobozzz
Copy link
Contributor Author

Chocobozzz commented Dec 6, 2019

Sorry for the late. I'll try to implement it this month :)

@Chocobozzz
Copy link
Contributor Author

Done, I put the option behind removeHTMLWhitespaces

Copy link
Contributor

@vperron vperron left a comment

Choose a reason for hiding this comment

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

agreed :) a few nit picks about the code style.

Also, could you please document this as an extra option in the README ? Or no one would think about it :)

After that i'll make you a contributor to get those future PRs faster !

src/extract.js Outdated Show resolved Hide resolved
src/extract.js Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Dec 10, 2019

Codecov Report

Merging #68 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage    98.9%   98.92%   +0.01%     
==========================================
  Files           5        5              
  Lines         366      371       +5     
  Branches       63       64       +1     
==========================================
+ Hits          362      367       +5     
  Misses          4        4
Impacted Files Coverage Δ
src/test-fixtures.js 100% <100%> (ø) ⬆️
src/extract.js 98.6% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d80874...c2076c6. Read the comment docs.

@Chocobozzz
Copy link
Contributor Author

Also, could you please document this as an extra option in the README ? Or no one would think about it :)

I tried to add a small section, don't hesitate to tell me if you want something different

@vperron vperron merged commit 0b719cb into Polyconseil:master Dec 10, 2019
@vperron
Copy link
Contributor

vperron commented Dec 10, 2019

Integrated and published under version 2.9.0 !

@vperron
Copy link
Contributor

vperron commented Dec 10, 2019

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants