Skip to content

Issue #22: Initial solution for toggling sorting profiles by percentage #42

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue #22: Initial solution for toggling sorting profiles by percentage #42

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions internal/model/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ type Model struct {
detectedPackageName string
requestedFiles map[string]bool
filteredLinesByFile map[string][]int
profilesLoaded []*cover.Profile
sortByAsc bool
Copy link
Owner

Choose a reason for hiding this comment

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

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 😼

Copy link
Author

Choose a reason for hiding this comment

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

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.


activeView viewName
helpState helpState
Expand Down Expand Up @@ -188,7 +190,12 @@ func (m *Model) onProfilesLoaded(profiles []*cover.Profile) (tea.Model, tea.Cmd)

if m.sortByCoverage {
sort.Slice(profiles, func(i, j int) bool {
return percentCovered(profiles[i]) < percentCovered(profiles[j])
if m.sortByAsc {
return percentCovered(profiles[i]) > percentCovered(profiles[j])
} else {
return percentCovered(profiles[i]) < percentCovered(profiles[j])
}

})
}

Expand Down Expand Up @@ -254,7 +261,20 @@ func (m *Model) onKeyPressed(key string) (tea.Model, tea.Cmd) {
return m, loadFile(adjustedFileName, item.profile)
}

return m, nil
return m, nil

//toggle on and inisiate sortByCoverage (default = Asc)
case "[":
m.sortByCoverage = true
m.sortByAsc = !m.sortByAsc
m.Update(m.profilesLoaded)
return m, nil

case "]":
m.sortByCoverage = false
m.sortByAsc = false
m.Update(m.profilesLoaded)
return m, nil

case "?":
m.toggleHelp()
Expand Down Expand Up @@ -328,6 +348,7 @@ func (m *Model) loadProfiles(codeRoot, profileFilename string) tea.Cmd {

finalProfiles = append(finalProfiles, p)
}
m.profilesLoaded = finalProfiles

return finalProfiles
}
Expand Down