search

maciejhirsz / logos

Create ridiculously fast Lexers
https://logos.maciej.codes
Apache License 2.0
2.71k stars 105 forks source link

fix(lib): correctly match expected tokens #378

Open jeertmans opened 4 months ago

jeertmans commented 4 months ago

PR inspired by https://github.com/maciejhirsz/logos/issues/265#issuecomment-1939245397.

Currently, this only adds new (failing) tests that should (hopefully) all pass after the fix has been committed.

@jameshurt, can you check this and explain how we can implement the fix your mentioned? If you prefer, feel free to create your own PR :-)

When trying to implement what you said, I still had a failing test, in css.rs, see:

running 3 tests
test css::test_word_spacing ... ok
test css::test_line_height ... ok
test css::test_letter_spacing ... FAILED

failures:

---- css::test_letter_spacing stdout ----
thread 'css::test_letter_spacing' panicked at /export/home/eertmans/repositories/logos/tests/src/lib.rs:101:9:
assertion `left == right` failed
  left: (Err(()), "42e", 21..24)
 right: (Ok(Number), "42", 21..23)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    css::test_letter_spacing
github-actions[bot] commented 4 months ago

Benchmark results:

group                                         before                                 changes
-----                                         ------                                 -------
count_ok/identifiers                          1.00   829.2±20.33ns   896.0 MB/sec    1.01    834.5±9.44ns   890.3 MB/sec
count_ok/keywords_operators_and_punctators    1.00      2.5±0.07µs   826.6 MB/sec    1.02      2.5±0.04µs   812.5 MB/sec
count_ok/strings                              1.00   535.1±19.24ns  1552.3 MB/sec    1.09    580.9±6.97ns  1430.0 MB/sec
iterate/identifiers                           1.00   827.0±11.57ns   898.4 MB/sec    1.01   838.1±57.41ns   886.4 MB/sec
iterate/keywords_operators_and_punctators     1.00      2.6±0.05µs   789.7 MB/sec    1.03      2.6±0.05µs   769.0 MB/sec
iterate/strings                               1.07   577.3±11.32ns  1438.7 MB/sec    1.00   537.2±26.59ns  1546.4 MB/sec
marcospb19 commented 4 months ago

Tests are failing so I'm assuming this is a draft not ready for reviewing yet.

jeertmans commented 4 months ago

Tests are failing so I'm assuming this is a draft not ready for reviewing yet.

Yes indeed, I am waiting for a response from the author of the comment mentioned above :-)

jeertmans commented 2 months ago

Note: the path from @elenakrittik should (if solves the issue) be cleaned up as it makes some function useless (like always returning None from switch?).