Fix distance_of_time_in_words "and" connector#4562
Conversation
**Why**: Before: "4 minutesand59 seconds" After: "4 minutes and 59 seconds"
config/locales/dotiw/en.yml
Outdated
| datetime: | ||
| dotiw: | ||
| two_words_connector: and | ||
| two_words_connector: " and " |
There was a problem hiding this comment.
I think our YAML normalization will trim/strip this 😬
I worked around this by adding the spaces in Ruby in a different file: https://github.com/18F/identity-idp/blob/master/app/presenters/account_reset/pending_presenter.rb#L17
There was a problem hiding this comment.
I think our YAML normalization will trim/strip this 😬
I worked around this by adding the spaces in Ruby in a different file: https://github.com/18F/identity-idp/blob/master/app/presenters/account_reset/pending_presenter.rb#L17
Hm! I don't see those normalizations happening locally, though I could see how that'd be an interesting challenge to deal with.
The current failures actually seem to be caused by a doubling-up of the spaces for specs associated with the file you mention. Should be as easy to fix as removing said workaround.
Not sure if it's something that changed in a more recent upgrade of the i18n-tasks dependency. It would seem to me as overly-opinionated of a normalization tool to forbid meaningful whitespace like this 🤔
There was a problem hiding this comment.
Another possibility is that previous attempts hadn't included the " quoting, in which case I'd expect the trimming behavior you describe to be applied to any excess whitespace in the YAML file.
There was a problem hiding this comment.
I don't remember the rationale or goal anymore... but we do some trailing whitespace trimming in our class:
https://github.com/18F/identity-idp/blob/master/lib/yaml_normalizer.rb#L50-L55
https://github.com/18F/identity-idp/blob/master/spec/lib/yaml_normalizer_spec.rb#L64-L73
There was a problem hiding this comment.
Ah, I'd missed the additional normalization we do beyond i18n-tasks. Yeah, I now see the problem you mention.
Thoughts:
- Should we avoid the trimming altogether? Or maybe at least apply trimming on the dumped output, rather than on the individual values?
- I've found some examples of using Psych combined with its
parse_streamand AST nodestyleattribute +SINGLE_QUOTED/DOUBLE_QUOTEDconstants to have better control on the parsing and output styles (example). Could consider for a refactor to give us better control to avoid trimming quoted values.
In the meantime, I think I'll revert back to a workaround like you originally proposed, since it seems those ideas may take a bit more effort to implement than I can afford now.
There was a problem hiding this comment.
Updated in c91c509 to pass option everywhere we use distance_of_time_in_words.
There was a problem hiding this comment.
Yeah I think a great alternative would be trim but only for unquoted strings or something but agree, definitely better to tackle in a separate PR
This reverts commit d48c8ea.
This reverts commit 291fb04.
**Why**: Not ideal, but since YAML normalization currently cannot tolerate string whitespace padding (even when quoted), we cannot include the spaces in the original YAML localization and instead must provide the option in usage. In the future, consider refactoring our YAML normalization to be more tolerant of intentional whitespace. See: https://github.com/18F/identity-idp/pull/4562/files#r552217136
2 participants
Why:
Before: "4 minutesand59 seconds"
After: "4 minutes and 59 seconds"