Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add elm syntax regression test #1227

Merged
merged 1 commit into from
Oct 4, 2020
Merged

Conversation

mdevlamynck
Copy link
Contributor

Adds an elm example as part of the #1213 regression test suite. This is the triangle example from https://elm-lang.org/examples. The original file comes from https://github.com/elm/elm-lang.org/ where the license can be found.

@sharkdp
Copy link
Owner

sharkdp commented Oct 4, 2020

The tests are failing. Could it be that you are not running the latest version of bat?

@mdevlamynck
Copy link
Contributor Author

Indeed, my system was running an old version of bat. The highlighting is even better now ^^

I'm wondering, maybe the test script shouldn't rely on the system installed bat, but instead on a locally built version?

@sharkdp
Copy link
Owner

sharkdp commented Oct 4, 2020

I'm wondering, maybe the test script shouldn't rely on the system installed bat, but instead on a locally built version?

I was wondering that too. We could run cargo run --release instead of bat, but that would potentially run a build first. It would also add a dependency on cargo (which is obviously a build depedency of bat, but isn't required for changes like this for now).

In the current version, the script gives control to the user on which version to use. We use this in our CI pipeline because there we want to run assets/create.sh first, then install bat, and then run the script, in order to test with the very latest syntax highlighting changes.

@sharkdp sharkdp merged commit 66c7aca into sharkdp:master Oct 4, 2020
@mdevlamynck mdevlamynck deleted the elm-syntax-test branch October 4, 2020 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants