feat: Introduce optional enhancers to casing function #884

nulltoken · 2020-01-03T18:21:26Z

Fixes #788
Checklist

Tests added / updated
Docs added / updated

Does this PR introduce a breaking change?

Yes
No

src/functions/casing.ts

nulltoken · 2020-01-03T18:33:39Z

@philsturgeon @P0lip This is an early push. Would you mind taking a look at the proposed interface to see it that would fit the need?

philsturgeon · 2020-01-03T18:58:42Z

src/functions/__tests__/casing.test.ts

+  target: unknown,
+  type: CasingType,
+  disallowDigits?: boolean,
+  separatorForCamelAndPascalCasing?: string,


Maybe it's less about making specific "seperators for camel and pascal", and more... "I would like to be able to enforce kebab casing for this URL, but ofc the URL will have slashes as well as kebab."

I've came up with a different approach where a new separator can be optionally defined to separate "words".

ie. in camel, with separator /, allowing digits
valid:

helloThere/iAm007

h/elloThere/iAm007

helloThere/iAm007

invalid:

helloThere/iAm0/07

hello/There/iAm007

helloTherei/Am007

Thoughts?

src/types/rule.ts

src/functions/casing.ts

nulltoken · 2020-01-05T15:23:51Z

src/functions/__tests__/casing.test.ts

+      );
+    });
+
+    test('allows advanced scenarios', () => {


@philsturgeon test cases from initial issue

src/functions/casing.ts

src/types/rule.ts

nulltoken · 2020-01-10T08:04:51Z

@philsturgeon @P0lip I've cleaned up the history, rebased and updated the doco. Would you please take another pass at this proposal?

src/functions/casing.ts

P0lip · 2020-01-10T09:28:58Z

src/functions/casing.ts

  }

-  return;
+  if (opts.allowLeadingSeparator !== undefined && opts.separator === undefined) {
+    throw new AssertionError({ message: "'allowLeadingSeparator' can only be valued when 'separator' is defined." });


Typings could be tweaked then. See my suggestion.
This is optional though.

P0lip · 2020-01-10T09:30:17Z

src/functions/casing.ts

+
+interface ICasingOptionsOverrides {
+  disallowDigits?: boolean;
+  separator?: string;


Suggested change

separator?: string;

interface ICasingOptionsOverridesWithSeperator {

disallowDigits?: boolean;

separator: string;

allowLeadingSeparator: boolean;

P0lip · 2020-01-10T09:30:55Z

src/functions/casing.ts

-  macro: /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*$/,
+export interface ICasingOptions extends ICasingOptionsOverrides {
+  type: CasingType;
+}


Suggested change

}

}

export type CasingOptions = ICasingOptions & ICasingOptionsOverrides | ICasingOptions & ICasingOptionsOverridesWithSeperator;

philsturgeon

This looks good once @P0lip's type tweaks are done. Great work!

P0lip

👍
Just need to have conflicts resolved and could also tweak typings a bit.

src/functions/__tests__/casing.test.ts

nulltoken · 2020-01-13T21:18:24Z

@P0lip I may need some help with regards to the proposed type tweakings.

Implementing the proposal makes indeed the interface cleaner. However, the TypeScript compiler prevents me from writing code like

  if (opts.allowLeadingSeparator !== undefined && opts.separator === undefined) {
    throw new AssertionError({ message: "'allowLeadingSeparator' can only be valued when 'separator' is defined." });
  }

because such code doesn't satisfy CasingOptions type declarations.

And although I could tweak it by throwing some anys at it or type guards, I'm not really sure it's really worth it. As this type declaration will not offer any real run time protection with regards to what the users could inject in their rulesets.

The current code is indeed much simpler but covers all the code execution branches.

As usual, iIf you feel those type tweakings should really be implemented, I'll work my way through them so that all the current tests keep on passing.

Thoughts?

Edit: FWIW, you initially mentioned they were optional and I didn't implement the suggestion for the reason described above.

nulltoken · 2020-01-13T21:43:15Z

@P0lip I gave some thought about it and I'm going to try and tweak the interface into something like

export interface ICasingOptions {
  type: CasingType;
  disallowDigits?: boolean;
  separator?: {
    char: string;
    allowLeading?: boolean;
  }
}

which I believe might actually convey more meaning than the initial interface I came out with (and might not need some elaborate type tweaking ;-) ).

nulltoken · 2020-01-13T22:06:25Z

@P0lip The new proposal has been pushed. What do you think about it?

src/functions/__tests__/casing.test.ts

P0lip

👍

nulltoken commented Jan 3, 2020

View reviewed changes

src/functions/casing.ts Outdated Show resolved Hide resolved

philsturgeon reviewed Jan 3, 2020

View reviewed changes

src/types/rule.ts Outdated Show resolved Hide resolved

nulltoken commented Jan 5, 2020

View reviewed changes

src/functions/casing.ts Outdated Show resolved Hide resolved

nulltoken commented Jan 5, 2020

View reviewed changes

nulltoken requested a review from P0lip January 6, 2020 22:35

P0lip reviewed Jan 6, 2020

View reviewed changes

src/functions/casing.ts Outdated Show resolved Hide resolved

src/functions/casing.ts Outdated Show resolved Hide resolved

src/functions/casing.ts Outdated Show resolved Hide resolved

src/functions/casing.ts Outdated Show resolved Hide resolved

philsturgeon reviewed Jan 7, 2020

View reviewed changes

src/types/rule.ts Outdated Show resolved Hide resolved

nulltoken mentioned this pull request Jan 8, 2020

chore: reorganize rule types declaration #898

Merged

4 tasks

nulltoken force-pushed the ntk/casing_separators branch from 5375e91 to 862a0a1 Compare January 10, 2020 08:01

nulltoken changed the title ~~[WIP] Introduce optional enhancers to casing function~~ feat: Introduce optional enhancers to casing function Jan 10, 2020

nulltoken requested review from philsturgeon and P0lip January 10, 2020 08:03

P0lip added the enhancement New feature or request label Jan 10, 2020

P0lip assigned nulltoken Jan 10, 2020

P0lip previously approved these changes Jan 10, 2020

View reviewed changes

nulltoken dismissed P0lip’s stale review via b72136b January 12, 2020 18:44

nulltoken force-pushed the ntk/casing_separators branch from 862a0a1 to b72136b Compare January 12, 2020 18:44

nulltoken requested a review from P0lip January 12, 2020 18:47

philsturgeon reviewed Jan 13, 2020

View reviewed changes

P0lip reviewed Jan 13, 2020

View reviewed changes

src/functions/__tests__/casing.test.ts Outdated Show resolved Hide resolved

nulltoken force-pushed the ntk/casing_separators branch from 6cc7b2a to a631e26 Compare January 13, 2020 21:19

nulltoken force-pushed the ntk/casing_separators branch from a631e26 to 03028c1 Compare January 13, 2020 22:04

nulltoken force-pushed the ntk/casing_separators branch from 03028c1 to df13c46 Compare January 13, 2020 22:07

feat: Introduce optional enhancers to casing function

56d9e10

nulltoken force-pushed the ntk/casing_separators branch from df13c46 to 56d9e10 Compare January 13, 2020 22:08

nulltoken commented Jan 13, 2020

View reviewed changes

src/functions/__tests__/casing.test.ts Show resolved Hide resolved

P0lip approved these changes Jan 13, 2020

View reviewed changes

P0lip merged commit 4bd5614 into develop Jan 13, 2020

P0lip deleted the ntk/casing_separators branch January 13, 2020 23:01

nulltoken mentioned this pull request Sep 9, 2020

Routes with parameters are never validates with casing rule #1327

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce optional enhancers to casing function #884

feat: Introduce optional enhancers to casing function #884

nulltoken commented Jan 3, 2020 •

edited by P0lip

Loading

nulltoken commented Jan 3, 2020

philsturgeon Jan 3, 2020

nulltoken Jan 5, 2020 •

edited

Loading

nulltoken Jan 5, 2020

nulltoken commented Jan 10, 2020

P0lip Jan 10, 2020

P0lip Jan 10, 2020

philsturgeon Jan 13, 2020

P0lip Jan 10, 2020

philsturgeon left a comment

P0lip left a comment

nulltoken commented Jan 13, 2020 •

edited

Loading

nulltoken commented Jan 13, 2020 •

edited

Loading

nulltoken commented Jan 13, 2020

P0lip left a comment

-  separator?: string;
+interface ICasingOptionsOverridesWithSeperator {
+  disallowDigits?: boolean;
+  separator: string;
+  allowLeadingSeparator: boolean;

-}
+}
+export type CasingOptions = ICasingOptions & ICasingOptionsOverrides | ICasingOptions & ICasingOptionsOverridesWithSeperator;

feat: Introduce optional enhancers to casing function #884

feat: Introduce optional enhancers to casing function #884

Conversation

nulltoken commented Jan 3, 2020 • edited by P0lip Loading

nulltoken commented Jan 3, 2020

philsturgeon Jan 3, 2020

Choose a reason for hiding this comment

nulltoken Jan 5, 2020 • edited Loading

Choose a reason for hiding this comment

nulltoken Jan 5, 2020

Choose a reason for hiding this comment

nulltoken commented Jan 10, 2020

P0lip Jan 10, 2020

Choose a reason for hiding this comment

P0lip Jan 10, 2020

Choose a reason for hiding this comment

philsturgeon Jan 13, 2020

Choose a reason for hiding this comment

P0lip Jan 10, 2020

Choose a reason for hiding this comment

philsturgeon left a comment

Choose a reason for hiding this comment

P0lip left a comment

Choose a reason for hiding this comment

nulltoken commented Jan 13, 2020 • edited Loading

nulltoken commented Jan 13, 2020 • edited Loading

nulltoken commented Jan 13, 2020

P0lip left a comment

Choose a reason for hiding this comment

nulltoken commented Jan 3, 2020 •

edited by P0lip

Loading

nulltoken Jan 5, 2020 •

edited

Loading

nulltoken commented Jan 13, 2020 •

edited

Loading

nulltoken commented Jan 13, 2020 •

edited

Loading