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

Remove error in src/commands/toggleSort.ts #2683

Open
trevinhofmann opened this issue Dec 5, 2024 · 4 comments
Open

Remove error in src/commands/toggleSort.ts #2683

trevinhofmann opened this issue Dec 5, 2024 · 4 comments
Labels
hold Pause development refactor Refactor without changing behavior

Comments

@trevinhofmann
Copy link
Collaborator

This error can now be removed:

// Show an error if the ranks do not match the sort condition.
// This is only needed for migrating to permasort, and can be removed after the migration is complete.
error: state => {
if (!state.cursor || isRoot(state.cursor)) return null
const simplePath = simplifyPath(state, state.cursor)
const id = head(simplePath)
const sortPreference = getSortPreference(state, id)
if (sortPreference.type === 'None') return null
const childrenSorted = getAllChildrenSorted(state, id)
const childrenRanked = getChildrenRanked(state, id)
return childrenSorted.length === childrenRanked.length &&
!childrenRanked.every((childRanked, i) => childRanked.id === childrenSorted[i].id)
? 'Ranks do not match sort condition'
: null
},
}

The question was originally raised here, and the error was confirmed to be ready for removal in a separate PR here.

@trevinhofmann trevinhofmann added the refactor Refactor without changing behavior label Dec 5, 2024
@trevinhofmann
Copy link
Collaborator Author

I've found a case which triggers this condition. I will add details shortly.

@trevinhofmann

This comment was marked as outdated.

@raineorshine
Copy link
Contributor

raineorshine commented Dec 6, 2024

Good catch! I will create a separate issue for the sort error (#2685).

The error function actually used the wrong path, which made it more confusing. Fixed in c277ca7.

@raineorshine
Copy link
Contributor

Still finding sorting errors, so keeping it in for now.

@cybersemics cybersemics locked and limited conversation to collaborators Jan 11, 2025
@cybersemics cybersemics unlocked this conversation Jan 11, 2025
@raineorshine raineorshine added the hold Pause development label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Pause development refactor Refactor without changing behavior
Projects
None yet
Development

No branches or pull requests

2 participants