Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Jasmin lexer text analysis #51

Merged
merged 2 commits into from Dec 12, 2020
Merged

Jasmin lexer text analysis #51

merged 2 commits into from Dec 12, 2020

Conversation

gandarez
Copy link
Member

@gandarez gandarez commented Dec 2, 2020

This PR ports pygments Jasmin text analysis to chroma/go. Original code can be found at: https://github.com/pygments/pygments/blob/master/pygments/lexers/jvm.py#L1608

Also I found useful to make sure the result never gets higer than 1.0 and added Min to it. Also the same implementation has been done for GDScript. I already opned a PR for Pygments.

@gandarez gandarez added the enhancement New feature or request label Dec 2, 2020
result += 0.6
}

return float32(math.Min(result, float64(1.0)))
Copy link

Choose a reason for hiding this comment

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

Returning a number > 1 might not do any harm. Maybe it is even intended here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't know. Let's see what happens with PR I submitted and if we don't get any response I'll change back to the original logic.

Copy link

Choose a reason for hiding this comment

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

Ok, let's wait for the pygments PR. But I would suggest to use our general approach for cases of uncertainties and stick to logic of wakatime python client for now.

We cannot go wrong with having the old pygments logic here, because this is what's currently in use in the wakatime python client. And I could imagine, that the pygments PR will hang for some time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed and already reverted the usage of min() to the python's logic. Also I'll follow up the PR I submitted to Pygments.

@gandarez
Copy link
Member Author

gandarez commented Dec 9, 2020

@dron22 rebased done.

@gandarez gandarez merged commit f727e12 into master Dec 12, 2020
@gandarez gandarez deleted the jasmin-lexer-text-analysis branch December 12, 2020 13:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants