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

fix panic when highlighter cache is empty #259

Merged
merged 1 commit into from
Sep 4, 2019

Conversation

keith-hall
Copy link
Collaborator

this fixes a panic which occurs when highlighting a JSON file using a recent version of JSON.sublime-syntax from the sublimehq/Packages repo. This bug was originally discovered at sharkdp/bat#644 - the steps for reproducing the panic are there.

Note, I haven't added a unit test for this yet, and am not sure when I will get chance to work on one... If someone else wants to take it on, please do.

@sharkdp
Copy link
Contributor

sharkdp commented Sep 3, 2019

Awesome, thank you for taking a look at this, @keith-hall!

Copy link
Collaborator

@robinst robinst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Owner

@trishume trishume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time right now to review wether the fact that the cache was being unwrapped is due to some other place breaking an invariant or just that I didn't consider a valid reason to have an empty cache. So I'll just trust that this is fine :P

@trishume trishume merged commit 64ac9bd into trishume:master Sep 4, 2019
@sharkdp
Copy link
Contributor

sharkdp commented Sep 21, 2019

It would be fantastic to get a patch release with this bugfix, as it prevents us from updating sublimhq/Packages in bat (sharkdp/bat#644).

(only if you find the time, I definitely don't want to bother you with downstream problems 😃)

@trishume
Copy link
Owner

@sharkdp Done, released as v3.3.0

@sharkdp
Copy link
Contributor

sharkdp commented Sep 22, 2019

@trishume Thank you so much!

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.

4 participants