Repository: bradymholt/cRonstrue
Pull request #342 (2025-03-25 14:37:45)

Add workaround for french periods & fix side effect with previous change
In #337 we discussed about "de le" should be "du" in French, this has been commited as "de" (in #338) for other cases but I encountered some other cases where "le" was missing for days of week crons.

This is a lightweight fix that works correctly but `src/expressionDescriptor.ts` contains a special case for french to replace "de le" to "du" and I cannot figure ou how to do that properly without editing all locales to add a custom method. I wanted to keep it simple.

Feel free to give some feedback or commit to this PR if you have something better :) Thanks

bradymholt: Thanks for the effort here!

I'm hoping we can use a solution that is more generic and avoid any hard-coding of locale referenced in the logic.

Suggestion:

In `src/i18n/locale.ts`, add:

```ts
/**
 * When `verbose` option is set to `false`, use this array to do any replacements to make the description more concise
 * For example:
 * return {
   "only in January": "in January",
   "every weekend day": "weekends"
   };
*/
conciseVerbosityReplacements?(): Record<string, string>;
```

In `src/i18n/locales/fr.ts`, add:

```ts
conciseVerbosityReplacements(){
    return {
      "de le": "du",
    };
  }
```

In `src/expressionDescriptor.ts`, add the following inside of `transformVerbosity`:

```ts
 if (this.i18n.conciseVerbosityReplacements) {
  for (const [key, value] of Object.entries(this.i18n.conciseVerbosityReplacements)) {
    description = description.replace(new RegExp(key, "g"), value);
  }
}
```
QuentiumYT: I agree that the hard coded way is not optimal, your solution looks better to me! I tried to do something like the 2 methods already in transformVerbosity but that was too specific. A record is a very clever solution.

I will add a commit that uses this system, just one question: does that mean I have to add this method to literally every locale implementation file?
bradymholt: No, the question mark suffix on the method in the interface
(conciseVerbosityReplacements?) marks it optional so you'll only need to
add it to French locale.

On Wed, Mar 26, 2025, 1:52 AM Quentium ***@***.***> wrote:

> I agree that the hard coded way is not optimal, your solution looks better
> to me! I tried to do something like the 2 methods already in
> transformVerbosity but that was too specific. A record is a very clever
> solution.
>
> I will add a commit that uses this system, just one question: does that
> mean I have to add this method to literally every locale implementation
> file?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bradymholt/cRonstrue/pull/342#issuecomment-2753401092>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAFZQA2OUK24ASETZOVP5GD2WI6A7AVCNFSM6AAAAABZXZX2ACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGQYDCMBZGI>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
> [image: QuentiumYT]*QuentiumYT* left a comment (bradymholt/cRonstrue#342)
> <https://github.com/bradymholt/cRonstrue/pull/342#issuecomment-2753401092>
>
> I agree that the hard coded way is not optimal, your solution looks better
> to me! I tried to do something like the 2 methods already in
> transformVerbosity but that was too specific. A record is a very clever
> solution.
>
> I will add a commit that uses this system, just one question: does that
> mean I have to add this method to literally every locale implementation
> file?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bradymholt/cRonstrue/pull/342#issuecomment-2753401092>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAFZQA2OUK24ASETZOVP5GD2WI6A7AVCNFSM6AAAAABZXZX2ACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGQYDCMBZGI>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>

QuentiumYT: This seems good, I added more choice tests (with comma) to check if specific french wording are correct. Except some `, et <xxx>` there are no real issue there :) thanks for your suggestion

Patch:
diff --git a/src/expressionDescriptor.ts b/src/expressionDescriptor.ts
index ae58895..7c62981 100644
--- a/src/expressionDescriptor.ts
+++ b/src/expressionDescriptor.ts
@@ -6,7 +6,7 @@ import { Locale } from "./i18n/locale";
 import { LocaleLoader } from "./i18n/localeLoader";
 
 export class ExpressionDescriptor {
-  static locales: { [name: string]: Locale } = {};
+  static locales: { [name: string]: Locale; } = {};
   static defaultLocale: string;
   static specialCharacters: string[];
 
@@ -713,6 +713,12 @@ export class ExpressionDescriptor {
       description = description.replace(new RegExp(`, ${this.i18n.everyHour()}`, "g"), "");
       description = description.replace(new RegExp(this.i18n.commaEveryDay(), "g"), "");
       description = description.replace(/\, ?$/, "");
+
+      if (this.i18n.conciseVerbosityReplacements) {
+        for (const [key, value] of Object.entries(this.i18n.conciseVerbosityReplacements())) {
+          description = description.replace(new RegExp(key, "g"), value);
+        }
+      }
     }
     return description;
   }
diff --git a/src/i18n/locale.ts b/src/i18n/locale.ts
index 86018ba..935ae34 100644
--- a/src/i18n/locale.ts
+++ b/src/i18n/locale.ts
@@ -3,6 +3,15 @@ export interface Locale {
   // TODO: These locale translations would be a good use for ES6 template strings except we sometimes concatenate multiple transactions together before
   //       doing the actual template replacement.
 
+  /**
+   * When `verbose` option is set to `false`, use this array to do any replacements to make the description more concise
+   * For example:
+   * return {
+      "only in January": "in January",
+      "every weekend day": "weekends",
+    };
+  */
+  conciseVerbosityReplacements?(): Record<string, string>;
   setPeriodBeforeTime?(): boolean;
   pm?(): string;
   am?(): string;
diff --git a/src/i18n/locales/fr.ts b/src/i18n/locales/fr.ts
index 8a092b5..87408a3 100644
--- a/src/i18n/locales/fr.ts
+++ b/src/i18n/locales/fr.ts
@@ -2,6 +2,12 @@
 
 import { Locale } from "../locale";
 export class fr implements Locale {
+  conciseVerbosityReplacements() {
+    return {
+      "de le": "du",
+    };
+  }
+  
   atX0SecondsPastTheMinuteGt20(): string|null {
     return null;
   }
@@ -146,7 +152,7 @@ export class fr implements Locale {
     return ", du %s au %s du mois";
   }
   commaOnDayX0OfTheMonth() {
-    return ", %s du mois";
+    return ", le %s du mois";
   }
   commaEveryHour() {
     return ", chaque heure";


Test patch:
diff --git a/test/i18n-fr.ts b/test/i18n-fr.ts
index bb3fd33..331a297 100644
--- a/test/i18n-fr.ts
+++ b/test/i18n-fr.ts
@@ -140,6 +140,40 @@ describe("Cronstrue (fr)", function () {
     });
   });
 
+  describe("choice", function () {
+    it("* * 3,8 * *", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 3 et 8 du mois");
+    });
+
+    it("* * * * 2,6", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, uniquement le mardi et samedi");
+    });
+
+    it("* * 3 * 2", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 3 du mois, et mardi");
+    });
+
+    it("* * 3 * 2,6", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 3 du mois, et mardi et samedi");
+    });
+
+    it("* * 3,8 * 2", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 3 et 8 du mois, et mardi");
+    });
+
+    it("* * 3,8 * 2,6", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 3 et 8 du mois, et mardi et samedi");
+    });
+
+    it("* * 1,3,5,7,9 * 2,4,6", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le 1, 3, 5, 7, et 9 du mois, et mardi, jeudi, et samedi");
+    });
+
+    it("0,30,59 0,12,23 1,3,5,7,9 3,6,9,12 2,4,6", function () {
+      assert.equal(cronstrue.toString(this.test?.title as string), "0, 30, et 59 minutes après l'heure, 00:00, 12:00, et 23:00, le 1, 3, 5, 7, et 9 du mois, et mardi, jeudi, et samedi, uniquement en mars, juin, septembre, et décembre");
+    });
+  });
+
   describe("last", function () {
     it("* * * * 4L", function () {
       assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, le dernier jeudi du mois");
@@ -167,11 +201,8 @@ describe("Cronstrue (fr)", function () {
       assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, toutes les 6 heures, à partir de 03:00");
     });
 
-    // ERROR: This is false but it can't be fixed properly for french language
-    // "à partir de 3" should be "à partir du 3"
-    // See https://github.com/bradymholt/cRonstrue/pull/338
     it("* * 3/6 * *", function () {
-      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, tous les 6 jours, à partir de 3 du mois");
+      assert.equal(cronstrue.toString(this.test?.title as string), "Toutes les minutes, tous les 6 jours, à partir du 3 du mois");
     });
 
     it("* * * 3/6 *", function () {