WIP: Functional rewrite #197

jehna · 2019-07-20T20:24:11Z

Work related to issue #196

This is a functional style rewrite from the v1.0 JSVerbalExpressions. The main difference is that the v1.0 had a builder-like interface:

const tester = VerEx()
    .startOfLine()
    .then('http')
    .maybe('s')
    .then('://')
    .maybe('www.')
    .anythingBut(' ')
    .endOfLine();

And the new version is just a bunch of functions that output regular expressions:

const tester = VerEx(
  startOfLine,
  "http",
  maybe("s"),
  "://",
  maybe("www."),
  anythingBut(" "),
  endOfLine
);

All of the functions output valid regular expressions, and all the constants are valid regular expressions. So for example or('foo', 'bar') outputs a normal regular expression /(?:foo)|(?:bar)/.

This branch is still a draft/WIP

The new version will be written directly with Typescript, so we'll start by having a simple, blank Typescript project base.

Extracted "simplifyExpression" helper function to its own file

Extracted `simpleExp` function so we can easily compile expressions that only add something to the input

Changed all functions to return regular expressions. This way we use regexp as the sanitized form and strings are always unsanitized

shreyasminocha · 2019-07-21T07:20:33Z

Apologies, didn't mean to mark as "ready for review", but wanted to leave a review.

shreyasminocha

Could we also move tests into their own directory test?

shreyasminocha · 2019-07-21T07:26:02Z

src/multiple.ts

+  minTimes?: number,
+  maxTimes?: number
+) {
+  const output = minTimes


Let's try to support:

multiple('foo') — foo*

multiple('foo', 2) — foo{2}

multiple('foo', 2, 5) — foo{2, 5}

multiple('foo', 2, Infinity) — foo{2,}

shreyasminocha · 2019-07-21T07:29:22Z

src/group.ts

+import { Expression } from "./types";
+import { compile } from "./verbalexpressions";
+
+export default function group(...inputs: Expression[]) {


It would be nice to have group support the following:

group('foo') — functionally identical to group.capturing

group.capturing('foo')

group.nonCapturing('foo')

shreyasminocha · 2019-07-21T07:31:20Z

src/anythingBut.ts

@@ -0,0 +1,4 @@
+import { simpleExp } from "./utils";
+
+const anythingBut = simpleExp<string>(exp => `[^${exp}]*`);


This deviates a bit from the old API, but I feel renaming this function to anyCharacterBut making it output [^${exp}] would make the API more intuitive.

shreyasminocha · 2019-07-21T07:31:40Z

src/anyOf.ts

@@ -0,0 +1,3 @@
+export default function anyOf(input: string) {


This deviates a bit from the old API, but I feel renaming this function to anyCharacterFrom would make the API more intuitive.

shreyasminocha · 2019-07-21T07:37:34Z

src/anyOf.ts

@@ -0,0 +1,3 @@
+export default function anyOf(input: string) {


I think this should accept an array of (characters | 2 length arrays). The two-length arrays would be a tuple of start and end characters. Here's an example:

anyOf(['a', 'b', 'c', ['0', '9'], ['A', 'Z']])

Implementation stub:

const charOrRange = string | [string, string]; export default function anyOf(input: Array<charOrRange>) { … }

Maybe we could force the strings in charOrRange to be of length one with fancier typescript constructs.

shreyasminocha · 2019-07-21T07:51:15Z

src/utils.ts

@@ -0,0 +1,27 @@
+import { Expression } from "./types";


Could we move each of these into their own files in a src/util folder?

shreyasminocha · 2019-07-21T07:51:30Z

src/types.ts

@@ -0,0 +1 @@
+export type Expression = RegExp | string | number;


Could we move each of these into their own files in a src/types folder?

shreyasminocha · 2019-07-21T07:58:17Z

I could add lookaround(), lookaround.ahead() and lookaround.behind(). lookaround() would be identical to lookaround.ahead().

shreyasminocha · 2019-07-21T07:59:23Z

Slightly offtopic, but Docz would be perfect for docs. I could work on those.

shreyasminocha · 2019-07-21T08:52:55Z

Rather than passing RegExps around, having SimpleExp etc, could we have a type called RawString, which will essentially be a string, but it'll help us distinguish built regex expressions from input strings?

shreyasminocha · 2019-07-21T08:54:49Z

Please let me know if you want me to implement any of the things I've pointed out. I'd be more than happy to contribute/split_the_work.

jehna · 2019-07-29T07:28:41Z

@shreyasminocha thank you for taking time to review! You have good points, and I'll address these comments as soon as possible.

I'd also like to hear your honest opinion about the high-level approach — how do you like using plain functions versus using a builder-like object to generate regular expressions? Do you think this this is the right direction for JSVerbalExpressions? Do you think these changes make the library simpler/better to use/more extensible — or the contrary?

I think now would be a good time to discuss these kind of high-level questions so we can still change things radically if better ideas arise

shreyasminocha · 2019-07-29T08:05:06Z

Do you think this this is the right direction for JSVerbalExpressions?

Yes, I think this is a good idea. However, since the semantics of usage will change, we should not be afraid to change the names of API methods where necessary to make the API more intuitive.

Do you think these changes make the library simpler/better to use/more extensible — or the contrary?

I think that they do make it more intuitive, more flexible and more powerful 🚀.

I'd also like to hear your honest opinion about the high-level approach — how do you like using plain functions versus using a builder-like object to generate regular expressions?

If I understand you correctly, I prefer the idea of plain functions (like the second example in #197) to a builder pattern.

On the implementation side of things, like I highlighted in the review, I think the functions should each return vanilla strings like [a-z] (or maybe a custom type—also essentially a string—to distinguish them from stuff that needs to be sanitized). VerEx would accept an arbitrary number of such strings, and generate a RegExp. Also, I don't think we should support mutability of the returned object. Maybe that would have the side-effect of allowing us to avoid calling the deprecated compile.

shreyasminocha · 2019-09-22T07:52:49Z

Hey @jehna.

I've done some work off your branch. I think the best way to go about this is to create a 2.0.0 branch on VerbalExpressions/JSVerbalExpressions and work there.

Update: Created a 2.0.0 branch as well as a 2.0.0 project.

I'm not sure if GitHub allows you to, but can you change the branch linked with this PR to the new one?

jehna · 2019-09-23T04:59:03Z

Great, thank you! Sorry for not having that much time to work on this project :/

I'm not sure if GitHub allows you to, but can you change the branch linked with this PR to the new one?

Unfortunately you need to close this one and open up another if you want to change the source branch. And that's okay for me :)

shreyasminocha · 2019-09-23T11:28:01Z

Discussion will continue in #198.

jehna added 14 commits July 19, 2019 21:41

Add empty Typescript base

c4d1946

The new version will be written directly with Typescript, so we'll start by having a simple, blank Typescript project base.

Add Jest

0c22c21

Add startOfLine and endOfLine constants

8e6b16b

Add "maybe" function

bcbee70

Extracted "simplifyExpression" helper function to its own file

Add or function

681a413

Add "anything" constant

5b92a93

Add "something" constant

e4e38d1

Add anythingBut and somethingBut

f7afdd6

Extracted `simpleExp` function so we can easily compile expressions that only add something to the input

Add oneOrMore

ac3bcd0

Add anyOf

63ecd43

Add multiple

7142f93

Add some complex test cases

bd3c82e

Add proper sanitizion for inputs

c7e5459

Changed all functions to return regular expressions. This way we use regexp as the sanitized form and strings are always unsanitized

Add capture groups

f67b826

jehna changed the title ~~Functional rewrite~~ WIP: Functional rewrite Jul 20, 2019

jehna mentioned this pull request Jul 20, 2019

Functional rewrite #196

Open

shreyasminocha marked this pull request as ready for review July 21, 2019 07:17

shreyasminocha reviewed Jul 21, 2019

View reviewed changes

shreyasminocha added this to the 2.0.0 milestone Jul 21, 2019

shreyasminocha mentioned this pull request Sep 23, 2019

Functional Rewrite (2.0.0) #198

Open

shreyasminocha closed this Sep 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Functional rewrite #197

WIP: Functional rewrite #197

jehna commented Jul 20, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019

shreyasminocha left a comment •

edited

Loading

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019

shreyasminocha Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019

shreyasminocha commented Jul 21, 2019 •

edited

Loading

jehna commented Jul 29, 2019

shreyasminocha commented Jul 29, 2019

shreyasminocha commented Sep 22, 2019 •

edited

Loading

jehna commented Sep 23, 2019

shreyasminocha commented Sep 23, 2019

		@@ -0,0 +1,4 @@
		import { simpleExp } from "./utils";

		const anythingBut = simpleExp<string>(exp => `[^${exp}]*`);

		@@ -0,0 +1,3 @@
		export default function anyOf(input: string) {

		@@ -0,0 +1 @@
		export type Expression = RegExp \| string \| number;

WIP: Functional rewrite #197

WIP: Functional rewrite #197

Conversation

jehna commented Jul 20, 2019 • edited Loading

shreyasminocha commented Jul 21, 2019

shreyasminocha left a comment • edited Loading

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019

Choose a reason for hiding this comment

shreyasminocha Jul 21, 2019 • edited Loading

Choose a reason for hiding this comment

shreyasminocha commented Jul 21, 2019 • edited Loading

shreyasminocha commented Jul 21, 2019 • edited Loading

shreyasminocha commented Jul 21, 2019

shreyasminocha commented Jul 21, 2019 • edited Loading

jehna commented Jul 29, 2019

shreyasminocha commented Jul 29, 2019

shreyasminocha commented Sep 22, 2019 • edited Loading

jehna commented Sep 23, 2019

shreyasminocha commented Sep 23, 2019

jehna commented Jul 20, 2019 •

edited

Loading

shreyasminocha left a comment •

edited

Loading

shreyasminocha Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019 •

edited

Loading

shreyasminocha commented Jul 21, 2019 •

edited

Loading

shreyasminocha commented Sep 22, 2019 •

edited

Loading