Skip to content

Conversation

friday
Copy link
Member

@friday friday commented Apr 17, 2022

All the actual functionality is exactly as implemented by @Henje in #1009, but this is rebased over #1014 which solved some of the issues with #1009. is_enabled() has also been completely rewritten to use ast (just checking the root node) instead of regex, which made it easier to prevent false positives.

Also added more tests.

It seems pretty solid to me after these changes.

Checklist

  • Verify that the test command ./ul test is passing (the CI server will check this if you don't)
  • Update the documentation according to your changes (when applicable)
  • Write unit tests for your changes (when applicable)

@Henje
Copy link
Contributor

Henje commented Apr 17, 2022

This looks great. I was not sure how to integrate my extension well, so this seems like the better solution. Thanks for your help.

@friday
Copy link
Member Author

friday commented Apr 17, 2022

This looks great. I was not sure how to integrate my extension well, so this seems like the better solution. Thanks for your help.

You did awesome work on your PR, but the calculator's was just not in a good state to extend with this type of functionality, which lead to the strange results.

@friday friday merged commit c0b9b88 into v6 Apr 17, 2022
@friday friday deleted the v6-henje-advanced-math branch April 17, 2022 20:53
This was referenced Apr 17, 2022
@friday friday added this to the 6.0.0 milestone Apr 17, 2022
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.

2 participants