4 ms·

(Separate comment addressing the library itself.) I find the naming of the classes off. Why not call X as Any? Why use nouns for some things, and verbs for o

(Separate comment addressing the library itself.) I find the naming of the classes off. Why not call X as Any? Why use nouns for some things, and verbs for others? I found Include and Exclude to be confusion. Inclusion and exclusion are relative to something. You include something with something else. I think Only and AnyBut would be better names. Why call it Seq when it's closer to a Python Range? Seq usually means sequentiality. Why not call Times as Repeat or Repetition? It didn't look like this had any compatibility with existing regexes. I can't parse an existing regex into this library. It looks like debugging could be a nightmare since it just passes things off to Python's re library. So if an error happens there, it will be tough to trace it to your original construction. All of this class hierarchy building seems prime to replace with algebraic data types, which would probably cut the code down to just a fraction of the size.

iogf 9y ago
I may add synonyms for those classes, so, it is just a matter of adding: Repeat = Times etc. you wouldnt use these structures in python code, you use them to build your regex then compile it and use the regex in your programs. It is possible to add some type safety and messages to help debugging which would be better than with natural regex syntax.
- iogf 9y ago
  Seq stands for a-z or 0-9 which can be used only in Include and Exclude. When you write Include(Seq('a', 'm')) it means you are including a character from that sequence in a given position of your pattern. So if you have Pattern(Include(Seq('a', 'm')), 'c') That is sort of building the following set of strings. ac bc cc . . . When you use Exclude, it would be like you are picking a char that is not belonging to the sequence, like in: Pattern(Exclude(Seq('0, 9')), c) then you would end up with a set like: ac bc cc . . . It means, a set whose first character of the elements arent digits.
  - masklinn 9y ago
    Seq is odd, to me Seq('a', 'm') says it matches 'am'. Why not overload X to support filtered forms instead? E.g X() is ".", X(from_, to_) is $from-$to, X(items) is [$items] and X(exclude) is [^$items]?
    - iogf 9y ago
      hmm, I think it is good, maybe p0 = X('a', 'z'). I like it.
      - masklinn 9y ago
        Plus then you get to more add convenient overloads e.g. (X('a') | X('z')), (~X('q')), X[1:7] (I guess slices would be ranges, and indexes would be explicit numbers of repetitions), etc…
        iogf 9y ago
        I think so i would ask you to open issues in crocs at github, so i can better keep track of suggestions and implement them.
Asooka 9y ago
I also think the naming is really bad and can be improved. The central problem is that you're using action words for a descriptive API. You should use something like "Repeating" instead of "Times". Also, the parameters to Times are distinctly un-pythonic. I would probably make it provide this API: def Repeating(pattern, *args, min=None, max=None, exact=None):... Repeating(P) => P* Repeating(P, a, b) and Repeating(P, min=a, max=b) => P{a, b} Repeating(P, n) and Repeating(P, exact=n) => P{n, n} Repeating(P, min=n) => P{n,} Repeating(P, max=n) => P{0, n} "Exclude" and "Include" suffer from the aforementioned problem of being procedural words for a declarative syntax. And they're easy to confuse with one another, having only two differing letters. And they don't make it clear that they match exactly one character. I would use "AnyBut" and "OneOf". Why the split between Group and NamedGroup? Just add a name parameter to Group! Seq as mentioned by parent is also problematic. Use Range or maybe Between or FromTo. To me FromTo flows the best, but Range is also ok. I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write Digit = FromTo('0', '9') CapitalLetter = FromTo('A', 'Z') POCode = CapitalLetter * 2 + Digit * 3 SmallLetter = FromTo('a', 'z') Letter = CapitalLetter or SmallLetter Name = CapitalLetter + Letter * (0, Inf) FirstName = Group(name='first', Name) LastName = Group(name='last', Name) FullName = FirstName + ' ' + LastName However, implemented badly that can hurt readability much more than having to write functions everywhere.
- masklinn 9y ago
  > I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write For repeat I think using indexing would make more sense though it might look somewhat unusual: Pattern[:] would be * Pattern[1:] would be + aka {1,} Pattern[5] would be {5} Pattern[m:n] would be {m,n} It also uses a single operator for all repetition operations which is nice.
  - deleted 9y ago
    [deleted]
- 9y ago

iogf 9y ago

I may add synonyms for those classes, so, it is just a matter of adding: Repeat = Times etc. you wouldnt use these structures in python code, you use them to build your regex then compile it and use the regex in your programs. It is possible to add some type safety and messages to help debugging which would be better than with natural regex syntax.

iogf 9y ago
Seq stands for a-z or 0-9 which can be used only in Include and Exclude. When you write Include(Seq('a', 'm')) it means you are including a character from that sequence in a given position of your pattern. So if you have Pattern(Include(Seq('a', 'm')), 'c') That is sort of building the following set of strings. ac bc cc . . . When you use Exclude, it would be like you are picking a char that is not belonging to the sequence, like in: Pattern(Exclude(Seq('0, 9')), c) then you would end up with a set like: ac bc cc . . . It means, a set whose first character of the elements arent digits.
- masklinn 9y ago
  Seq is odd, to me Seq('a', 'm') says it matches 'am'. Why not overload X to support filtered forms instead? E.g X() is ".", X(from_, to_) is $from-$to, X(items) is [$items] and X(exclude) is [^$items]?
  - iogf 9y ago
    hmm, I think it is good, maybe p0 = X('a', 'z'). I like it.
    - masklinn 9y ago
      Plus then you get to more add convenient overloads e.g. (X('a') | X('z')), (~X('q')), X[1:7] (I guess slices would be ranges, and indexes would be explicit numbers of repetitions), etc…
      - iogf 9y ago
        I think so i would ask you to open issues in crocs at github, so i can better keep track of suggestions and implement them.

Asooka 9y ago

I also think the naming is really bad and can be improved. The central problem is that you're using action words for a descriptive API. You should use something like "Repeating" instead of "Times". Also, the parameters to Times are distinctly un-pythonic. I would probably make it provide this API: def Repeating(pattern, *args, min=None, max=None, exact=None):... Repeating(P) => P* Repeating(P, a, b) and Repeating(P, min=a, max=b) => P{a, b} Repeating(P, n) and Repeating(P, exact=n) => P{n, n} Repeating(P, min=n) => P{n,} Repeating(P, max=n) => P{0, n} "Exclude" and "Include" suffer from the aforementioned problem of being procedural words for a declarative syntax. And they're easy to confuse with one another, having only two differing letters. And they don't make it clear that they match exactly one character. I would use "AnyBut" and "OneOf". Why the split between Group and NamedGroup? Just add a name parameter to Group! Seq as mentioned by parent is also problematic. Use Range or maybe Between or FromTo. To me FromTo flows the best, but Range is also ok. I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write Digit = FromTo('0', '9') CapitalLetter = FromTo('A', 'Z') POCode = CapitalLetter * 2 + Digit * 3 SmallLetter = FromTo('a', 'z') Letter = CapitalLetter or SmallLetter Name = CapitalLetter + Letter * (0, Inf) FirstName = Group(name='first', Name) LastName = Group(name='last', Name) FullName = FirstName + ' ' + LastName However, implemented badly that can hurt readability much more than having to write functions everywhere.

masklinn 9y ago
> I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write For repeat I think using indexing would make more sense though it might look somewhat unusual: Pattern[:] would be * Pattern[1:] would be + aka {1,} Pattern[5] would be {5} Pattern[m:n] would be {m,n} It also uses a single operator for all repetition operations which is nice.
- deleted 9y ago
  [deleted]
9y ago