-
Notifications
You must be signed in to change notification settings - Fork 57
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/non breakable spaces #760
Conversation
@@ -260,7 +261,7 @@ class JiraService { | |||
throw new RuntimeException(message) | |||
} | |||
|
|||
return new JsonSlurperClassic().parseText(response.getBody()) | |||
return new JsonSlurperClassic().parseText(StringCleanup.removeCharacters(response.getBody())) |
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.
@s2oBCN you should reconsider this interface. The characters to be removed depend on the scope. In a JiraService, we want to remove characters we don't want from Jira. But the StringCleanup class holds these characters. I suggest to refactor to StringUtil.removeCharacters(text, characters)
and let the JiraService store the unwanted characters. Better coupling IMHO.
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 idea here is that it doesn't depends on JiraService, depends on what we don't want in our pdfs, so if tomorrow we have another interface we will use the same. Basilly, decouple code.
And by the other hand don't add more code to JiraService
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.
@s2oBCN but the character '\u00A0' comes from Jira and is cleaned up in the JiraService. Hard-coding this into a generic StringCleanup class is not clean.
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.
@s2oBCN, @metmajer The StringUtil.removeCharacter has been refactor to include the characters we want to remove and moved the characters Map (valueToRemove: NewValue f.e. nonbreakablespace: space) to the JiraService, so the class is reusable and the characters belongs to the calling instance, in this case to the JiraService.
Sorry for the number of commits as there was a misundertanding of the comments, with the squash they will dissapear
* Remove no-break space from json output of JiraServices
fix non breakable spaces in reports by removing special caracters