Skip to content

English doesn't have a "day after tomorrow" word#2889

Merged
MichMich merged 3 commits intoMagicMirrorOrg:developfrom
MikeBishop:no_day_after
Jul 28, 2022
Merged

English doesn't have a "day after tomorrow" word#2889
MichMich merged 3 commits intoMagicMirrorOrg:developfrom
MikeBishop:no_day_after

Conversation

@MikeBishop
Copy link
Contributor

English doesn't have a word for the day after the day after today. "In 2 days" doesn't align with the behavior around relative formatting for non-full-day events -- that uses "today," "tomorrow," and then goes to days of the week.

Having played around with different formats, I've concluded that "In 2 days" is an odd-ball that shows up here only(?) for English. Many languages have a word for that; obviously the code should continue to accommodate those languages.

The simplest "fix" then is simply to drop this term from en.json, as the code already handles the DAYAFTERTOMORROW field not being defined for a given language. So let's just let English be such a language.

@MikeBishop
Copy link
Contributor Author

MikeBishop commented Jul 25, 2022

Hm. So I see that the tests enforce that all translated items must be in both the base language and all other languages. While that's admirable for coverage, in this instance the code explicitly checks whether the term exists and moves on properly if it's absent.

Is there a way to explicitly allow this translation not to exist, perhaps by having it present but with a null value? (Edit: From a cursory inspection of translator.js, it appears to check for presence of the key, not that the value is truthy.)

@khassel
Copy link
Collaborator

khassel commented Jul 25, 2022

looking into the translation test: en is used as the "complete" language (with all keys assigned).

So if you delete one key, we have to use another language. Tested with de:

change in this line translations.en to translations.de and

change in this line "en" to "de"

and the test will work again. May add some comments why this was changed ...

@codecov-commenter
Copy link

Codecov Report

Merging #2889 (0c1e5ea) into develop (2d15e4f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2889   +/-   ##
========================================
  Coverage    63.82%   63.82%           
========================================
  Files            9        9           
  Lines          293      293           
========================================
  Hits           187      187           
  Misses         106      106           

Help us with your feedback. Take ten seconds to tell us how you rate us.

@MikeBishop
Copy link
Contributor Author

That appears to have worked -- thank you!

@MichMich
Copy link
Collaborator

Thanks! Sorry for the slow merge. Was enjoying a sunny holiday. :)

@MichMich MichMich merged commit aaa9042 into MagicMirrorOrg:develop Jul 28, 2022
@MikeBishop
Copy link
Contributor Author

No worries at all! (For anyone's general edification, English does technically have such a word -- "overmorrow" -- but it hasn't been in common use since Middle English.)

@MikeBishop MikeBishop deleted the no_day_after branch July 28, 2022 14:44
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.

5 participants