Issue #22: Initial solution for toggling sorting profiles by percentage #42
Conversation
Picked two arbitrary keybinds "[" toggles on sortedByPercentage and will toggle back and forth between Asc and Desc order. "]" turns off sorting by percentage. Need to add "instructions" to the key map. i.e. "[ toggle sort by percentage"
There was a problem hiding this comment.
This is amazing, thank you! I left one comment for a discussion about the general direction we can take this thing from here in the future.
Of course, after we decide about it, we will need to add a test to this thing, I can help you with that if you get stuck.
| @@ -78,6 +78,8 @@ type Model struct { | |||
| detectedPackageName string | |||
| requestedFiles map[string]bool | |||
| filteredLinesByFile map[string][]int | |||
| profilesLoaded []*cover.Profile | |||
| sortByAsc bool | |||
There was a problem hiding this comment.
Instead of using two boolean flags, I would suggest using an "enum" type with values like SortByPercentage, SortByStatements, SortByName (default one), and a boolean flag that represents asc and desc values. Then, there will be a single key, not two keys as you suggested, that toggle over these 3 values:
- starting with order by name by default (if the flag is not set)
- toggle to sort by percentage
- optionally, if you with to implement as well, toggle to sort by uncovered statements
- toggle to sort by name again
So the point is that the user only uses a single key to toggle through all possible sorting states.
And then, there is another key that toggles between ascending and descending order. My thoughts were to use s key for sorting mode toggle, and - or ! or z to reverse the order.
Of course if we drop the future requirement to implement 3 different sort modes (at least) your solution with a boolean flag is perfect, so if you don't feel like changing it, that's completely OK 😼
There was a problem hiding this comment.
I like your idea, and it will get me some practice using enums. Ill mess around with this and start working on some tests. Ill do frequent PRs so you can help guide me. Thanks for the feedback.
Codecov ReportBase: 86.12% // Head: 84.79% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #42 +/- ##
==========================================
- Coverage 86.12% 84.79% -1.33%
==========================================
Files 10 10
Lines 807 822 +15
==========================================
+ Hits 695 697 +2
- Misses 94 107 +13
Partials 18 18
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Picked two arbitrary keybinds "[" toggles on sortedByPercentage and will toggle back and forth between Asc and Desc order. "]" turns off sorting by percentage. Need to add "instructions" to the key map. i.e. "[ toggle sort by percentage"
Please give feedback on possible better solutions or best practices.
NOT YET COMPLETED:
I need to still implement adding the key bindings to the "register" of key maps. I could use some tips on this.
Thanks!