Navigation Menu

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

Fix default messages extractor bug with line break escaping #22140

Conversation

LeanidShutau
Copy link
Contributor

Bug:
en.json generated by default messages extraction tool contains not escaped line breaks if defaultMessage contains more then one line break.

Reason:
String.replace(string, string) is not recursive.
String.replace(RegExp, string) should be used instead.

How to test:
Jest test with described case has been added to utils.test.js.

@LeanidShutau LeanidShutau added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Project:i18n labels Aug 17, 2018
@LeanidShutau LeanidShutau self-assigned this Aug 17, 2018
Copy link

@yankouskia yankouskia left a comment

Choose a reason for hiding this comment

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

LGTM, just 1 note

expect(
formatJSString(`Test
multiline
string`)
Copy link

Choose a reason for hiding this comment

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

let's please add test with something like:

`text with 2


line-breaks and 2 \n\n
\n\n

for example
`

?

(and maybe toMatchSnapshot will be easier?)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@LeanidShutau LeanidShutau added this to Review in I18n Aug 20, 2018
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM

@LeanidShutau LeanidShutau merged commit 96df714 into elastic:master Aug 20, 2018
@LeanidShutau LeanidShutau deleted the fix/default-messages-extractor-linebreak-bug branch August 20, 2018 13:21
@LeanidShutau LeanidShutau moved this from Review to Backporting in I18n Aug 20, 2018
LeanidShutau added a commit to LeanidShutau/kibana that referenced this pull request Aug 21, 2018
…22140)

* Fix default messages extractor bug with line break escaping

* Change test case
LeanidShutau added a commit that referenced this pull request Aug 21, 2018
…22211)

* Fix default messages extractor bug with line break escaping

* Change test case
@LeanidShutau
Copy link
Contributor Author

6.x/6.5: 75a1769

@LeanidShutau LeanidShutau moved this from Backporting to Done in I18n Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported bug Fixes for quality problems that affect the customer experience Project:i18n Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.5.0
Projects
I18n
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants