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

Update hotkey.lua: add isActive #3608

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

muescha
Copy link
Contributor

@muescha muescha commented Feb 29, 2024

add isActive

See:

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['pr-fix', 'pr-change', 'pr-feature', 'pr-maintenance']

@muescha
Copy link
Contributor Author

muescha commented Feb 29, 2024

TODO:

  • add docs if accepted
  • should it be better as function like htm:isActive()?

Copy link

View Test Results

2 tests   2 ✔️  0s ⏱️
2 suites  0 💤
1 files    0

Results for commit d78aeef.

@muescha
Copy link
Contributor Author

muescha commented Feb 29, 2024

i can not see why the test are failing :(

@cmsj cmsj added the pr-feature Pull Request implementing a feature label Aug 5, 2024
@cmsj
Copy link
Member

cmsj commented Aug 5, 2024

@muescha ignore the CI tests, they're horribly broken at the moment. I think this feature makes sense to add, so if you'd like to go ahead with it, I like your idea that an :isActive() method would be a better choice overall (so the precise implementation is hidden and can be changed later). With docs and that method, this can merge.

@muescha
Copy link
Contributor Author

muescha commented Aug 9, 2024

did you mean "hiding implementation" like this in 7e389a0

  • removing from metatable
  • renaming value to _isActive
  • creating :isActive() method

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.63%. Comparing base (36c6495) to head (7e389a0).
Report is 817 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3608      +/-   ##
==========================================
+ Coverage   27.61%   27.63%   +0.01%     
==========================================
  Files         180      191      +11     
  Lines       36181    43465    +7284     
==========================================
+ Hits         9993    12012    +2019     
- Misses      26188    31453    +5265     

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature Pull Request implementing a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants