Implement splitSelector in pure reason

[ulrikstrid]

This is a start to implement the splitSelector function in pure Reason and addresses #9.

Do you want it to work if compiled to native or is it still OK if it only compiles to JavaScript?

Any formatting change is from refmt that I don't know how to turn off.

[threepointone]

could you rebase against master to resolve the formatting changes?

[ulrikstrid]

That doesn't seem to solve it (I forked master recently) I have asked in the discord if there is a way to no refmt the whole file when changing 1 line.

[ulrikstrid]

I managed to solve it by just staging that line, not sure why I didn't do that at first

[threepointone]

it's fine if it compiles to javascript (for now), but anything in specific that prevents this from being pure reason? I'm fairly new to it, so wouldn't know

[thangngoc89]

@threepointone the regex is JS only.

[ulrikstrid]

Yepp, only compiles to JS. If built with bsb-native we can create a wrapper around JS regexp and ocaml-re and conditionally bring in a underlying implementation.

A probably better solution would be to make another tokenizer that doesn't rely on regex.

[ulrikstrid]

So I have this ugly implementation that doesn't rely on regex. Are there more complex examples then the one mentioned in the issue?

[kitten]

@ulrikstrid I noticed that you're allocating a lot of lists and are using some nested linear operations. I haven't quite read through what is required (sry), but I'd like to give it a try as well :wink: :

let splitSelector = (selector: string) : array(string) => {
  let res = [||];
  let size = String.length(selector);

  let rec explode = (start: int, length: int, depth: int) => {
    let index = start + length;
    let isPastEnd = index >= size;

    if (isPastEnd && length > 0) {
      Js.Array.push(res, String.sub(selector, start, length));
      res
    } else if (isPastEnd) {
      res
    } else {
      switch (String.unsafe_get(selector, start + length), depth) {
      | (',', 0) => {
        Js.Array.push(res, String.sub(selector, start, length));
        explode(start + length + 1, 0, 0)
      }
      | ('(', _) => explode(start, length + 1, depth + 1)
      | (')', _) => explode(start, length + 1, depth - 1) /* TODO: could throw on paren-mismatch here */
      | _ => explode(start, length + 1, depth)
      }
  };

  explode(0, 0, 0)
};

(not the best I can do, but I think replacing the regex logic with a recursive char "scanner" is always neat)

[ulrikstrid]

@philpl implementation looks a lot better than my "brute force" solution. But we should probably avoid the Js.Array.push as the goal is to be able to compile to native.

And we need more test cases as both our solutions seems pretty easy and the JS implementation looks kind of scary. Here is one that my implementation does not handle: '& :matches(h1, h2, h3, h4, h5, h6)'

[kitten]

@ulrikstrid The Js.Array.push is of course easy to swap out 👍 consider the snippet just a small start please :wink:

Here is one that my implementation does not handle: '& :matches(h1, h2, h3, h4, h5, h6)'

I'm currently working on a CSS parser project that is written in Reason as well (https://github.com/philpl/sweetsour-parser); It's unrelated for now, but I can expand the snippet based on my work there. Do we need to support nested selectors (ignoring compound selectors I presume)?

Edit: seems that the function also needs to skip over strings which I forgot 😆 shouldn't be too hard to add though

[ulrikstrid]

Updated to use a slightly modified version of @philpl implementation and changed the test case to have more commas in the parenthesis.

The snapshots seems to change when using that implementation but might just be that I did something stupid somewhere so I did not commit it.

[ulrikstrid]

I don't have the bandwidth to finish this, closing