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

Extend SU.collatedSort for complex table sorting #2105

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

Omikhleia
Copy link
Member

@Omikhleia Omikhleia commented Sep 10, 2024

I noticed when working on #2082 : Our SU.collatedSort() works on list of strings only:

local names = {"Charlie", "Bob", "Alice"}
SU.collatedSort(names)
--  gives: {"Alice", "Bob", "Charlie"}

My own bad at the time (#1632)... but we can't use it to sort structured (key-value) tables with a comparison callback, as one would do with table.sort().

Of course there's a workaround, but it's a bit annoying (comparing for string equality, then performing a second ICU comparison for ordering)

So this PR is a proposal for supporting it, as:

-- Sort by age then name
local namesAndAges = { {name="Charlie", age=25}, {name="Bob", age=30}, {name="Alice", age=25} }
SU.collatedSort(namesAndAges, nil, function (a, b, stringCompare)
  if a.age < b.age then return true end
  if a.age > b.age then return false end
  return stringCompare(a.name, b.name) < 0
end)
-- gives { {name="Alice", age=25}, {name="Charlie", age=25}, {name="Bob", age=30} }

-- Sort by name then year
local namesAndYears = { {name="Alice", year=2005}, {name="Charlie", year=1995}, {name="Bob", year=1990}, {name="Alice", year=1995} }
SU.collatedSort(namesAndYears, nil, function (a, b, stringCompare)
  local nameCompare = stringCompare(a.name, b.name)
  if nameCompare < 0 then return true end
  if nameCompare > 0 then return false end
  return a.year < b.year
end)
-- gives { {name="Alice", year=1995}, { name = "Alice", year = 2005 }, {name="Bob", year=1990}, {name="Charlie", year=1995} }

In other terms:

  • Be less clever in the C code and just return -1, 0, 1 for (less, equal, greater)
  • Use a closure to pass ICU-enabled string comparison to the user callback.

@Omikhleia Omikhleia requested a review from alerque as a code owner September 10, 2024 21:36
@Omikhleia Omikhleia self-assigned this Sep 10, 2024
@Omikhleia Omikhleia added the enhancement Software improvement or feature request label Sep 10, 2024
@Omikhleia Omikhleia added this to the v0.15.6 milestone Sep 10, 2024
@Omikhleia Omikhleia force-pushed the refactor-collated-sort branch from 104a39c to 102219a Compare September 10, 2024 21:50
@Omikhleia Omikhleia force-pushed the refactor-collated-sort branch from 102219a to 3854be8 Compare September 10, 2024 21:53
@Omikhleia
Copy link
Member Author

Omikhleia commented Sep 10, 2024

Oops sorry for the multiple push force... Was on wrongly rebased branch :)

@alerque alerque merged commit 7650113 into sile-typesetter:master Oct 1, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Software improvement or feature request
Projects
Development

Successfully merging this pull request may close these issues.

2 participants