Skip to content

fix(taglist): for property filter#4084

Open
cnjhb wants to merge 1 commit into
awesomeWM:masterfrom
cnjhb:taglist
Open

fix(taglist): for property filter#4084
cnjhb wants to merge 1 commit into
awesomeWM:masterfrom
cnjhb:taglist

Conversation

@cnjhb

@cnjhb cnjhb commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

table filter interfere with property filter, so we need to remove table filter from the instance.

In practical terms, this means we cannot modify the filter property of a taglist instance.

@codecov

codecov Bot commented Mar 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.48%. Comparing base (f1388f9) to head (42011d1).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4084      +/-   ##
==========================================
+ Coverage   90.45%   90.48%   +0.03%     
==========================================
  Files         941      942       +1     
  Lines       60390    60402      +12     
  Branches     1145     1145              
==========================================
+ Hits        54628    54657      +29     
+ Misses       5252     5238      -14     
+ Partials      510      507       -3     
Files with missing lines Coverage Δ
lib/awful/widget/taglist.lua 89.73% <100.00%> (+3.09%) ⬆️
tests/test-taglist.lua 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cnjhb cnjhb changed the title fix(taglist): for filter fix(taglist): for property filter Mar 13, 2026
@Aire-One

Aire-One commented Mar 15, 2026

Copy link
Copy Markdown
Member

The fix seems reasonable at first glance, but we do a lot of these gears.table.crush in our constructor-functions. There should be a lot of other properties wrongly copied this way in many modules. Applying this rawset to remove them afterward would just not be maintainable.

A better solution would be to basically refactor the module pattern we use and clearly separate the module level from instance level stuff. I'm currently experimenting Teal. The type system it brings would help a lot in this scenario. The big migration, in addition to being a massive work would probably be not welcomed considering the current status of the project.

@cnjhb

cnjhb commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

The fix seems reasonable at first glance, but we do a lot of these gears.table.crush in our constructor-functions. There should be a lot of other properties wrongly copied this way in many modules. Applying this rawset to remove them afterward would just not be maintainable.

A better solution would be to basically refactor the module pattern we use and clearly separate the module level from instance level stuff. I'm currently experimenting Teal. The type system it brings would help a lot in this scenario. The big migration, in addition to being a massive work would probably be not welcomed considering the current status of the project.

It’s true—it would be much better if the language supported object-oriented programming.
I’ve used Penlight’s class before, but they weren’t ideal either.

@cnjhb

cnjhb commented Mar 17, 2026

Copy link
Copy Markdown
Contributor Author

@Aire-One So how should we fix this?

table filter interfere with property filter, so we need to remove table filter from the instance.

@actionless actionless left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks! mb additionally some note should be added to the docstring

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.

3 participants