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

[sheets] fix splitcell to handle attribute/text pairs #2020

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

ajkerrigan
Copy link
Collaborator

@ajkerrigan ajkerrigan commented Sep 1, 2023

Have splitcell account for receiving a list of (attr, text) tuples rather than a list of text lines.

Return a list of (attr, line) for each wrapped line of text. The intent is to have _splitlines() play nicely with the displayer_generic() changes here.

Open questions:

  • Are there cases where _splitcell() will still get a list of strings rather than a list of (attr, text) pairs?
  • Is there a better/cleaner way to handle this?
  • Is there a better place to handle this?

Closes #2019

Unrelated drive-by fix: Looks like we were trying to sum a generator here when counting deferred changes during a commit-sheet command, and that was blowing up vgit tests. Noooope, let #2009 handle that.

Have splitcell account for receiving a list of (attr, text) tuples
rather than a list of text lines.

Return a list of (attr, line) for each wrapped line of text.
@ajkerrigan ajkerrigan changed the title [sheets] fix splitcell to handle attribute/text displayer tuples [sheets] fix splitcell to handle attribute/text tuples Sep 1, 2023
@ajkerrigan ajkerrigan changed the title [sheets] fix splitcell to handle attribute/text tuples [sheets] fix splitcell to handle attribute/text pairs Sep 1, 2023
@anjakefala
Copy link
Collaborator

Btw, the vgit test fix is here! #2009

The reason summing the generator blows up is because sum = aggregators::vsum.

@ajkerrigan
Copy link
Collaborator Author

Btw, the vgit test fix is here! #2009

The reason summing the generator blows up is because sum = aggregators::vsum.

Ahhh thanks! I didn't see anything that looked obviously related, I'll remove the drive-by piece of this then 🙈

@saulpw
Copy link
Owner

saulpw commented Sep 1, 2023

This looks good to me, @ajkerrigan! Thanks for taking care of it.

@saulpw saulpw merged commit e891ba4 into saulpw:develop Sep 1, 2023
14 of 22 checks passed
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.

v for expanding lines doesn't work
3 participants