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.

Sign up for GitHub

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

Jump to bottom

Localisation issues in sk.js (Slovak) #831

Closed
tukusejssirs opened this issue Mar 10, 2020 · 37 comments
Closed

Localisation issues in sk.js (Slovak) #831

tukusejssirs opened this issue Mar 10, 2020 · 37 comments

Comments

@tukusejssirs
Copy link
Contributor

Related #302 #4

There are some issues in sk.js:

  • ordinal: n => `${n}º`, → ordinal: n => `${n}.`,:
    • there should a dot after ordinal numbers;
  • L: 'DD.MM.YYYY', → L: 'DD. MM. YYYY',:
    • according to STN 01 6910 (in Slovak), the spaces after each dot in the date is prefered, but the spaces can be omitted, therefore this is not an issue per se;
  • weekdays: 'Nedeľa_Pondelok_Utorok_Streda_Štvrtok_Piatok_Sobota'.split('_'), → weekdays: 'nedeľa_pondelok_utorok_streda_štvrtok_piatok_sobota'.split('_'),
  • weekdaysShort: 'Ne_Po_Ut_St_Št_Pi_So'.split('_'), → weekdaysShort: 'ne_po_ut_st_št_pi_so'.split('_'),.

Then there is as an issue with the the numbers of and declension of the nouns (see also #302). The grammatical rules in Slovak are quite similar to those in Czech, therefore I have just copied the plural() and translate() functions from there and replaced the nouns with the Slovak ones. I would open a PR with this, but I am not a good coder, therefore this must be reviewed.

All in all, here is the proposed content of sk.js. I hope I did not made any mistakes. I have added some comments to make it easier to check if the words are really those that should be assigned to the particular key.

sk.js 
// Slovak [sk]
import dayjs from 'dayjs'

function plural(n) {
  return (n > 1) && (n < 5) && (~~(n / 10) !== 1) // eslint-disable-line
}

/* eslint-disable */ 
function translate(number, withoutSuffix, key, isFuture) {
  const result = `${number} `
  switch (key) {
    case 's': // a few seconds / in a few seconds / a few seconds ago
      // in a few seconds / a few seconds ago
      return (withoutSuffix || isFuture) ? 'pár sekúnd' : 'pár sekundami'
    case 'm': // a minute / in a minute / a minute ago
      return withoutSuffix ? 'minúta' : (isFuture ? 'minútu' : 'minútou')
    case 'mm': // 9 minutes / in 9 minutes / 9 minutes ago
      if (withoutSuffix || isFuture) {
        // 2-4 minutes / 5+ minutes
        return result + (plural(number) ? 'minúty' : 'minút')
      }
      // 2+ minutes ago
      return `${result}minútami`
    case 'h': // an hour / in an hour / an hour ago
      return withoutSuffix ? 'hodina' : (isFuture ? 'hodinu' : 'hodinou')
    case 'hh': // 9 hours / in 9 hours / 9 hours ago
      if (withoutSuffix || isFuture) {
        // 2-4 hours / 5+ hours
        return result + (plural(number) ? 'hodiny' : 'hodín')
      }
      // 2+ hours ago
      return `${result}hodinami`
    case 'd': // a day / in a day / a day ago
      // in a day / a day ago
      return (withoutSuffix || isFuture) ? 'deň' : 'dňom'
    case 'dd': // 9 days / in 9 days / 9 days ago
      if (withoutSuffix || isFuture) {
        // 2-4 days / 5+ days
        return result + (plural(number) ? 'dni' : 'dní')
      }
      // 2+ days ago
      return `${result}dny`
    case 'M': // a month / in a month / a month ago
      // in a month / a month ago
      return (withoutSuffix || isFuture) ? 'mesiac' : 'mesiacom'
    case 'MM': // 9 months / in 9 months / 9 months ago
      if (withoutSuffix || isFuture) {
        // 2-4 months / 5+ months
        return result + (plural(number) ? 'mesiace' : 'mesiacov')
      }
      // 2+ months ago
      return `${result}mesiacmi`
    case 'y': // a year / in a year / a year ago
      // in a year / a year ago
      return (withoutSuffix || isFuture) ? 'rok' : 'rokom'
    case 'yy': // 9 years / in 9 years / 9 years ago
      if (withoutSuffix || isFuture) {
        // 2-4 years / 5+ years
        return result + (plural(number) ? 'roky' : 'rokov')
      }
      // 2+ year ago
      return `${result}rokmi`
  }
}
/* eslint-enable */

const locale = {
  name: 'sk',
  weekdays: 'nedeľa_pondelok_utorok_streda_štvrtok_piatok_sobota'.split('_'),
  weekdaysShort: 'ne_po_ut_st_št_pi_so'.split('_'),
  weekdaysMin: 'ne_po_ut_st_št_pi_so'.split('_'),
  months: 'január_február_marec_apríl_máj_jún_júl_august_september_október_november_december'.split('_'),
  monthsShort: 'jan_feb_mar_apr_máj_jún_júl_aug_sep_okt_nov_dec'.split('_'),
  weekStart: 1,
  relativeTime: {
    future: 'o %s',
    past: 'pred %s',
    s: 'niekoľko sekúnd',
    m: 'minúta',
    mm: '%d minút',
    h: 'hodina',
    hh: '%d hodín',
    d: 'deň',
    dd: '%d dní',
    M: 'mesiac',
    MM: '%d mesiacov',
    y: 'rok',
    yy: '%d rokov'
  },
  ordinal: n => `${n}.`,
  formats: {
    LT: 'H:mm',
    LTS: 'H:mm:ss',
    L: 'DD. MM. YYYY',
    LL: 'D. MMMM YYYY',
    LLL: 'D. MMMM YYYY H:mm',
    LLLL: 'dddd D. MMMM YYYY H:mm'
  }
}

dayjs.locale(locale, null, true)

export default locale

PS—@prantlf, can I ask you to check the translate() function? Thanks in advance! 😃
@tukusejssirs tukusejssirs mentioned this issue Mar 10, 2020
Closed
@iamkun
Copy link
Owner

iamkun commented Mar 11, 2020 •
edited
Loading

Thanks.

It will be great if you could send us a PR to fix this.

And there will be a patch release this weekend.

plus, I'd like to keep L: 'DD.MM.YYYY' the same as moment.js and not to change it.

@tukusejssirs
Copy link
Contributor Author

Okay, I’ll tend it now.

plus, I'd like to keep L: 'DD.MM.YYYY' the same as moment.js and not to change it.

Okay, I accept that. However, can’t we add another variable (or whatever you call it, like the L) for DD. MM. YYYY?

@iamkun
Copy link
Owner

iamkun commented Mar 12, 2020

You can use https://day.js.org/docs/en/plugin/update-locale to suit your need in your project easily/

@tukusejssirs
Copy link
Contributor Author

Thanks. Laste question: should I open a PR against the dev branch?

@iamkun
Copy link
Owner

iamkun commented Mar 12, 2020

Yes, from the dev branch, please.

And you can check test/locale/cs.test.js as a reference to make the test pass with 100% coverage.

@tukusejssirs
Copy link
Contributor Author

And you can check test/locale/cs.test.js as a reference to make the test pass with 100% coverage.

I’ll do my best.

I cs.test.js, the following constant has some comments which might need some corrections. I know that the comments mean nothing to dayjs nor moment, therefore the test still passes, but I think the comments should be fixed.

What do you think?

  const T = [
    [44.4, 'second'], // a few seconds  ← Are `44.4s` considered _a few seconds_?
    [89.5, 'second'], // a minute       ← Are `89.5s` seconds _a minute_?
    // ...
    [43, 'minute'], // 44 minutes       ← I don’t think that `43` = `44`.
    [45, 'minute'], // an hour          ← I don’t think that `45m` = `1h`.
    // ...
    [18, 'month'] // 2 years            ← I don’t think that `18mo` = `2y`.
  ]

@iamkun
Copy link
Owner

iamkun commented Mar 12, 2020

Ha, that's the relative time result in English. And 44 minutes -> should be 43 minutes.

Fell free to fix it if you like 😁

@tukusejssirs
Copy link
Contributor Author

Okay. Just to confirm: only 44 minutes should be changed to 43 minutes, other constants are treated as relative time, correct?

@iamkun
Copy link
Owner

iamkun commented Mar 12, 2020 •
edited
Loading

Yes, cheers. You can skip these comments in your test file if you like.

@iamkun
Copy link
Owner

iamkun commented Mar 12, 2020

ref2 https://github.com/moment/moment/blob/develop/locale/sk.js

@tukusejssirs
Copy link
Contributor Author

I see. In order to fix dayjs I need to fix moment. 😩

@tukusejssirs
Copy link
Contributor Author

Anyway, does dayjs support the setting of the first day of week? See below the snippet from moment’ sk.js.

        week : {
            dow : 1, // Monday is the first day of the week.
            doy : 4  // The week that contains Jan 4th is the first week of the year.
        }

@tukusejssirs
Copy link
Contributor Author

I have open moment/moment#5408 issue and created moment/moment#5409 PR, but there are some Travis issues I could not fix.

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

Yes, https://day.js.org/docs/en/customization/customization weekStart, yearStart

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

Plus, how about we just keep the same result as moment.js at first, to get CI passed with 100% coverage, and release it in this week.

After that, we could keep fixing this minor error.

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

And you can run npm test to see test status and coverage easily.

@tukusejssirs
Copy link
Contributor Author 

  yearStart: 4, // OPTIONAL, the week that contains Jan 4th is the first week of the year.

Is 4 the default? The ISO week numbering is used in Slovakia, therefore if 4 is default, then no change is needed in here, otherwise I’d add it to sk.js.

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020 •
edited
Loading

I'll update this to make yearStart 4 default in this version, too. So, no change needed.

@tukusejssirs
Copy link
Contributor Author

I have run npm test which the changed value, but the following test does not pass:

 FAIL  test/plugin/utc-utcOffset.test.js
  ● utc startOf

    expect(received).toBe(expected) // Object.is equality
    
    Expected value to be:
      1568213999999
    Received:
      1568217599999

      94 |   expect(moment(d).utc().utcOffset(480).endOf('day')
      95 |     .valueOf())
    > 96 |     .toBe(dayjs(d).utc().utcOffset(480).endOf('day')
      97 |       .valueOf())
      98 | 
      99 |   expect(moment(d).utc().utcOffset(480).endOf('day')
      
      at Object.<anonymous> (test/plugin/utc-utcOffset.test.js:96:6)

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

Please add yearStart 4, sorry for my previous reply.

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

wired 😬

@tukusejssirs
Copy link
Contributor Author

And now, when I wanted to fix another l10n (I have missed translating dny to dňami), I cannot commit:

$ git commit -am 'Fix `dny` issue'

/home/ts/git/c10n/dayjs/src/index.js
  1:20  error  Missing file extension "js" for "./constant"   import/extensions
  2:15  error  Missing file extension "js" for "./utils"      import/extensions
  3:16  error  Missing file extension "js" for "./locale/en"  import/extensions

/home/ts/git/c10n/dayjs/src/locale/sk.js
  71:23  error  Multiple spaces found before '// Should be `...'  no-multi-spaces

/home/ts/git/c10n/dayjs/src/plugin/advancedFormat/index.js
  1:32  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/buddhistEra/index.js
  1:32  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/isoWeek/index.js
  1:25  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/localizedFormat/index.js
  1:32  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/quarterOfYear/index.js
  1:25  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/relativeTime/index.js
  1:20  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/utc/index.js
  1:44  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/plugin/weekOfYear/index.js
  1:29  error  Missing file extension "js" for "../../constant"  import/extensions

/home/ts/git/c10n/dayjs/src/utils.js
  1:20  error  Missing file extension "js" for "./constant"  import/extensions

/home/ts/git/c10n/dayjs/test/comparison.test.js
  2:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/constructor.test.js
  2:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/display.test.js
  3:19  error  Missing file extension "js" for "../src"            import/extensions
  4:16  error  Missing file extension "js" for "../src/locale/th"  import/extensions
  5:8   error  Missing file extension "js" for "../src/locale/ja"  import/extensions

/home/ts/git/c10n/dayjs/test/get-set.test.js
  3:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/locale/cs.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/cs"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/et.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/et"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/fi.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/fi"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/keys.test.js
  3:19  error  Missing file extension "js" for "../../src"  import/extensions

/home/ts/git/c10n/dayjs/test/locale/pl.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/pl"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/ru.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/ru"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/sk.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/sk"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/sv.test.js
  2:19  error  Missing file extension "js" for "../../src"                        import/extensions
  3:28  error  Missing file extension "js" for "../../src/plugin/advancedFormat"  import/extensions
  4:8   error  Missing file extension "js" for "../../src/locale/sv"              import/extensions

/home/ts/git/c10n/dayjs/test/locale/uk.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/uk"            import/extensions

/home/ts/git/c10n/dayjs/test/locale/zh-cn.test.js
  3:19  error  Missing file extension "js" for "../../src"               import/extensions
  4:8   error  Missing file extension "js" for "../../src/locale/zh-cn"  import/extensions

/home/ts/git/c10n/dayjs/test/locale/zh.test.js
  1:19  error  Missing file extension "js" for "../../src"                        import/extensions
  2:28  error  Missing file extension "js" for "../../src/plugin/advancedFormat"  import/extensions
  3:24  error  Missing file extension "js" for "../../src/plugin/weekOfYear"      import/extensions

/home/ts/git/c10n/dayjs/test/locale.test.js
  3:19  error  Missing file extension "js" for "../src"            import/extensions
  4:16  error  Missing file extension "js" for "../src/locale/es"  import/extensions

/home/ts/git/c10n/dayjs/test/manipulate.test.js
  3:19  error  Missing file extension "js" for "../src"               import/extensions
  4:8   error  Missing file extension "js" for "../src/locale/zh-cn"  import/extensions
  5:8   error  Missing file extension "js" for "../src/locale/ar"     import/extensions

/home/ts/git/c10n/dayjs/test/parse.test.js
  3:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/advancedFormat.test.js
  3:19  error  Missing file extension "js" for "../../src"                        import/extensions
  4:28  error  Missing file extension "js" for "../../src/plugin/advancedFormat"  import/extensions
  5:24  error  Missing file extension "js" for "../../src/plugin/weekOfYear"      import/extensions
  6:22  error  Missing file extension "js" for "../../src/plugin/weekYear"        import/extensions
  7:8   error  Missing file extension "js" for "../../src/locale/zh-cn"           import/extensions

/home/ts/git/c10n/dayjs/test/plugin/badMutable.test.js
  3:19  error  Missing file extension "js" for "../../src"                    import/extensions
  4:24  error  Missing file extension "js" for "../../src/plugin/badMutable"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/zh-cn"       import/extensions

/home/ts/git/c10n/dayjs/test/plugin/buddhistEra.test.js
  3:19  error  Missing file extension "js" for "../../src"                     import/extensions
  4:25  error  Missing file extension "js" for "../../src/plugin/buddhistEra"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/calendar.test.js
  3:19  error  Missing file extension "js" for "../../src"                  import/extensions
  4:22  error  Missing file extension "js" for "../../src/plugin/calendar"  import/extensions
  5:18  error  Missing file extension "js" for "../../src/locale/zh-cn"     import/extensions

/home/ts/git/c10n/dayjs/test/plugin/customParseFormat.test.js
  3:19  error  Missing file extension "js" for "../../src"                           import/extensions
  4:31  error  Missing file extension "js" for "../../src/plugin/customParseFormat"  import/extensions
  5:16  error  Missing file extension "js" for "../../src/locale/uk"                 import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/zh-cn"              import/extensions

/home/ts/git/c10n/dayjs/test/plugin/dayOfYear.test.js
  3:19  error  Missing file extension "js" for "../../src"                   import/extensions
  4:23  error  Missing file extension "js" for "../../src/plugin/dayOfYear"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isBetween.test.js
  2:19  error  Missing file extension "js" for "../../src/index"                   import/extensions
  3:23  error  Missing file extension "js" for "../../src/plugin/isBetween/index"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isLeapYear.test.js
  2:19  error  Missing file extension "js" for "../../src"                    import/extensions
  3:24  error  Missing file extension "js" for "../../src/plugin/isLeapYear"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isMoment.test.js
  2:19  error  Missing file extension "js" for "../../src"                  import/extensions
  3:22  error  Missing file extension "js" for "../../src/plugin/isMoment"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isoWeek.test.js
  3:19  error  Missing file extension "js" for "../../src"                 import/extensions
  4:21  error  Missing file extension "js" for "../../src/plugin/isoWeek"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isoWeeksInYear.test.js
  2:19  error  Missing file extension "js" for "../../src"                        import/extensions
  3:28  error  Missing file extension "js" for "../../src/plugin/isoWeeksInYear"  import/extensions
  4:24  error  Missing file extension "js" for "../../src/plugin/isLeapYear"      import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isSameOrAfter.test.js
  2:19  error  Missing file extension "js" for "../../src"                       import/extensions
  3:27  error  Missing file extension "js" for "../../src/plugin/isSameOrAfter"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/isSameOrBefore.test.js
  2:19  error  Missing file extension "js" for "../../src"                        import/extensions
  3:28  error  Missing file extension "js" for "../../src/plugin/isSameOrBefore"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/localeData.test.js
  3:19  error  Missing file extension "js" for "../../src"                         import/extensions
  4:24  error  Missing file extension "js" for "../../src/plugin/localeData"       import/extensions
  5:29  error  Missing file extension "js" for "../../src/plugin/localizedFormat"  import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/zh-cn"            import/extensions

/home/ts/git/c10n/dayjs/test/plugin/localizedFormat.test.js
  3:19  error  Missing file extension "js" for "../../src"                         import/extensions
  4:16  error  Missing file extension "js" for "../../src/locale/es"               import/extensions
  5:18  error  Missing file extension "js" for "../../src/locale/zh-cn"            import/extensions
  6:29  error  Missing file extension "js" for "../../src/plugin/localizedFormat"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/minMax.test.js
  2:19  error  Missing file extension "js" for "../../src"                import/extensions
  3:20  error  Missing file extension "js" for "../../src/plugin/minMax"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/quarterOfYear.test.js
  3:19  error  Missing file extension "js" for "../../src"                       import/extensions
  4:27  error  Missing file extension "js" for "../../src/plugin/quarterOfYear"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/relativeTime.test.js
  3:19  error  Missing file extension "js" for "../../src"                      import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/relativeTime"  import/extensions
  5:17  error  Missing file extension "js" for "../../src/plugin/utc"           import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/ru"            import/extensions

/home/ts/git/c10n/dayjs/test/plugin/toArray.test.js
  3:19  error  Missing file extension "js" for "../../src"                 import/extensions
  4:21  error  Missing file extension "js" for "../../src/plugin/toArray"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/toObject.test.js
  3:19  error  Missing file extension "js" for "../../src"                  import/extensions
  4:22  error  Missing file extension "js" for "../../src/plugin/toObject"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/updateLocale.test.js
  3:19  error  Missing file extension "js" for "../../src"                         import/extensions
  4:26  error  Missing file extension "js" for "../../src/plugin/updateLocale"     import/extensions
  5:29  error  Missing file extension "js" for "../../src/plugin/localizedFormat"  import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/zh-cn"            import/extensions

/home/ts/git/c10n/dayjs/test/plugin/utc-utcOffset.test.js
  3:19  error  Missing file extension "js" for "../../src"             import/extensions
  4:17  error  Missing file extension "js" for "../../src/plugin/utc"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/utc.test.js
  3:19  error  Missing file extension "js" for "../../src"                           import/extensions
  4:17  error  Missing file extension "js" for "../../src/plugin/utc"                import/extensions
  5:31  error  Missing file extension "js" for "../../src/plugin/customParseFormat"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin/weekday.test.js
  3:19  error  Missing file extension "js" for "../../src"                 import/extensions
  4:21  error  Missing file extension "js" for "../../src/plugin/weekday"  import/extensions
  5:8   error  Missing file extension "js" for "../../src/locale/zh-cn"    import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/ar"       import/extensions

/home/ts/git/c10n/dayjs/test/plugin/weekOfYear.test.js
  3:19  error  Missing file extension "js" for "../../src"                        import/extensions
  4:24  error  Missing file extension "js" for "../../src/plugin/weekOfYear"      import/extensions
  5:28  error  Missing file extension "js" for "../../src/plugin/advancedFormat"  import/extensions
  6:8   error  Missing file extension "js" for "../../src/locale/en-gb"           import/extensions

/home/ts/git/c10n/dayjs/test/plugin/weekYear.test.js
  3:19  error  Missing file extension "js" for "../../src"                    import/extensions
  4:22  error  Missing file extension "js" for "../../src/plugin/weekYear"    import/extensions
  5:24  error  Missing file extension "js" for "../../src/plugin/weekOfYear"  import/extensions

/home/ts/git/c10n/dayjs/test/plugin.test.js
  2:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/query.test.js
  2:19  error  Missing file extension "js" for "../src"  import/extensions

/home/ts/git/c10n/dayjs/test/timezone.test.js
  3:19  error  Missing file extension "js" for "../src"             import/extensions
  4:17  error  Missing file extension "js" for "../src/plugin/utc"  import/extensions

/home/ts/git/c10n/dayjs/test/utils.test.js
  1:19  error  Missing file extension "js" for "../src/utils"  import/extensions

✖ 134 problems (134 errors, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

pre-commit: 
pre-commit: We've failed to pass the specified git pre-commit hooks as the `lint`
pre-commit: hook returned an exit code (1). If you're feeling adventurous you can
pre-commit: skip the git pre-commit hooks by adding the following flags to your commit:
pre-commit: 
pre-commit:   git commit -n (or --no-verify)
pre-commit: 
pre-commit: This is ill-advised since the commit is broken.

@iamkun
Copy link
Owner

iamkun commented Mar 13, 2020

looks like your es-lint issue?

@tukusejssirs
Copy link
Contributor Author

I run npm install in the repo root in order to make npm test work. Since then only, these errors started to pop out.

I’ve just run npm install eslint, it was updated, but the above-mentioned git commit error persists.

$ npm install eslint
npm WARN deprecated circular-json@0.3.3: CircularJSON is in maintenance only, flatted is its successor.
npm WARN rollup-plugin-babel@4.4.0 requires a peer of rollup@>=0.60.0 <3 but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.11 (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.11: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

+ eslint@4.19.1
updated 1 package and audited 31231 packages in 23.842s

25 packages are looking for funding
  run `npm fund` for details

found 78 vulnerabilities (69 low, 8 moderate, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details

@tukusejssirs
Copy link
Contributor Author

When I run rm -rf coverage/ node_modules/ package-lock.json in the repo root, and then run git commit, it has output a different error:

.git/hooks/pre-commit: line 2: ./node_modules/pre-commit/hook: No such file or directory

Therefore, I have removed .git/hooks/pre-commit, then I could commit my changes (btw, I have just squashed my changes).

Anyway, here’s the output of .git/hooks/pre-commit:

#!/bin/bash
./node_modules/pre-commit/hook
RESULT=$?
[ $RESULT -ne 0 ] && exit 1
exit 0

@tukusejssirs
Copy link
Contributor Author

The test/plugin/utc-utcOffset.test.js test still fails.

@iamkun iamkun mentioned this issue Mar 13, 2020
Merged
@iamkun
Copy link
Owner

iamkun commented Mar 14, 2020

#833

@iamkun iamkun closed this as completed Mar 14, 2020
@tukusejssirs
Copy link
Contributor Author

Plus, how about we just keep the same result as moment.js at first, to get CI passed with 100% coverage, and release it in this week.

After that, we could keep fixing this minor error.

Moment seems to be unmaintained, see moment/moment#5405. Therefore we might wait for the fix in vain.

@iamkun
Copy link
Owner

iamkun commented Mar 18, 2020

can we give them 1 month first 😬 , then we could update it ourself

@tukusejssirs
Copy link
Contributor Author

tukusejssirs commented Apr 20, 2020 •
edited
Loading

can we give them 1 month first grimacing , then we could update it ourself

The month is up and there is no pulse in the Moment repo (only new open issues and PRs). I think we should not rely on Moment output anymore as there are many unfixed bugs (just open their issues and PRs to see what I mean). I think we should fix those issues in dayjs.

I think that we/I should open a separate issue here to address the Moment comparison removal from dayjs tests. However, I won’t be able to help you out much in that area (I am not a pro coder; I am rather a scripter and ‘composer’). I’ll help you if I can and have enough time that I can devote to dayjs.

Update: Here’s an issue that was opened on 6 March 2020 that asks if Moment is still maintained: moment/moment#5405

@iamkun
Copy link
Owner

iamkun commented Apr 21, 2020

Yes, I do agree. Let's fix it on our side. 😁

@tukusejssirs
Copy link
Contributor Author

How would you like it do it? 😃

@iamkun
Copy link
Owner

iamkun commented Apr 21, 2020 •
edited
Loading

You can create a pull request with the change, and I can help update the test accordingly. Just leave a comment with the correct result.

@tukusejssirs tukusejssirs mentioned this issue May 14, 2020
Closed
@tukusejssirs
Copy link
Contributor Author

tukusejssirs commented May 14, 2020 •
edited
Loading

@iamkun, sorry I could not find time to contribute the fix yet.

In the meanwhile, one of the team members of Moment responded in moment/moment#5405 if Moment is still maintaned and also there were some new commits made.

On the other hand, @k2s user pointed out that my implementation is not entirely correct. Basically, both o and za prepositions are corrent, but there are rules when to use which one (source in Slovak):

  • za is used when we talk about a period during which something is done (like He cooked it in an hour.);
  • o is used when we talk about a point in time (like She’s back in a minute.).

Then that user proposed a 'solution': .replace('za', 'o'). But that is not even a workaround as it leaves the output partially correct, although reversed [the current output not correct for o (but correct for za) and if one chooses to use to 'solution', they would get correct output for o, but not for za.].

I have no idea how to deal with this issue.

PS - It seems that the Moment devs are currently opposed to the correction of this issue, but I hope it'll change.

@iamkun
Copy link
Owner

iamkun commented May 15, 2020

@tukusejssirs there's might another way, just make your own locale file at this time.
https://day.js.org/docs/en/customization/customization

@tukusejssirs
Copy link
Contributor Author

Thanks. But I have troubles with the algorithm: how can I determine (without sentence/context) if in [a period of time] is a period during which is something done (then za should be used); if it is a period after which is something done (and therefore, current_time + period = the_referred_point_in_time; then o should be used)?

@iamkun
Copy link
Owner

iamkun commented May 15, 2020

@tukusejssirs https://day.js.org/docs/en/customization/relative-time#additional-token-processing

You can leave the future field future : '%s', only. And leave the logic to Additional token processing function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tukusejssirs @iamkun