trishume / syntect

Rust library for syntax highlighting using Sublime Text syntax definitions.
https://docs.rs/syntect
MIT License
1.85k stars 130 forks source link

Update `regex-syntax` to 0.8. #499

Closed CGMossa closed 7 months ago

CGMossa commented 7 months ago

I have updated the regex-syntax dependencies to 0.8. I don't have a way to test this further than cargo test --no-fail-fast, which showed the same tests failing on master as on this. So it is the same in that regard.

CGMossa commented 7 months ago

I've added the errors from both before upgrading regex-syntax and after.

test_fail_master.txt test_fail_updated_regex_syntax.txt

CGMossa commented 7 months ago

@keith-hall thanks for your response on the other issue. I promise you, I just checked again, cargo test fails on master. I even tried cargo test --no-fail-fast and it fails a lot.

CGMossa commented 7 months ago

I've had to run

git submodule init
git submodule update

and that helped with a bunch of the tests, locally. I see that CI has submodule: true, which is why I had different sets of failing tests on top of CI. But the tests still fail on master-branch for me locally.

CGMossa commented 7 months ago

I've found out that public_api.text needs to be updated by hand. This is done by setting env variable to UPDATE_EXPECT=1, that then generates a new public_api.txt.

According to the text, it "expects" nightly-2023-08-25, so I made a rust-toolchain.toml, and set it to

[toolchain]
channel = "nightly-2023-08-25"
CGMossa commented 7 months ago

Alright! I've rebased with master, hopefully this will make CI pass.

CGMossa commented 7 months ago

It works!

Details

``` PS C:\Users\minin\Documents\GitHub\syntect> cargo test --test public_api Compiling syntect v5.1.0 (C:\Users\minin\Documents\GitHub\syntect) warning: unused import: `serde_json::Value::Array as SettingsArray` --> src\highlighting\settings.rs:7:9 | 7 | pub use serde_json::Value::Array as SettingsArray; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default warning: unused import: `serde_json::Value::Object as SettingsObject` --> src\highlighting\settings.rs:8:9 | 8 | pub use serde_json::Value::Object as SettingsObject; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: `syntect` (lib) generated 2 warnings (run `cargo fix --lib -p syntect` to apply 2 suggestions) Finished test [unoptimized + debuginfo] target(s) in 10.92s Running tests\public_api.rs (C:/cargo_target_dir\debug\deps\public_api-82f2a8627bd89d28.exe) running 1 test Documenting syntect v5.1.0 (C:\Users\minin\Documents\GitHub\syntect) Finished dev [unoptimized + debuginfo] target(s) in 5.23s test public_api ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 6.57s ```

Enselic commented 7 months ago

Thanks! Let's merge.

The other CI failure looks like a bug in the public-api crate that I happen to maintain. I'll look into it.