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

Miscellaneous improvements #151

Closed
oscbyspro opened this issue Aug 12, 2022 · 9 comments
Closed

Miscellaneous improvements #151

oscbyspro opened this issue Aug 12, 2022 · 9 comments
Labels
brrr such code, much quality enhancement improvements n' stuff
Milestone

Comments

@oscbyspro
Copy link
Owner

I’m rather content with the state of the project, but there are a couple of things I’d like to improve, but that are not significant enough to warrant separate issues. So, instead, I’ll collect them here.

@oscbyspro oscbyspro added the brrr such code, much quality label Aug 12, 2022
@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 12, 2022

Pattern where capacity == 0 (aka the useless pattern)

The hidden() method should hide all characters, rather than hiding none. It behaves like this because the reduction function looks for the first placeholder characters, does not find one, then stops at the end index (meaning that the effect is the same as the first placeholder being the end index, which is false). Solving this should also remove the need for special case selection on 100% formatting.


A solution has been introduced in 2be0c99

@oscbyspro oscbyspro added this to the v5.0.0 milestone Aug 12, 2022
@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 12, 2022

Apparently, I forgot to override Snapshot/distance(from:to:) so it’s still O(n).


Added in: d7dd5b5. If you look at Collection and BidirectionalCollection, you will find that a custom implementation of distance(from:to:) is enough to override both count and underestimatedCount, and that the isEmpty default is OK.

@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 13, 2022

Lazy views improve readability

It’s easier to reason about the complexity of code when laziness is explicit. So I think it would be better if snapshot.nonvirtuals were instead snapshot.lazy.nonvirtuals(). With two extensions, you could write:

snapshot.nonvirtuals()
snapshot.lazy.nonvirtuals()

A follow up to this (and an actual improvement) would be adding a Proposal.Lazy:

proposal.merged().nonvirtuals()
proposal.lazy.merged().nonvirtuals() // this option does not yet exist

Added in: e4708c9. It removes an O(n) operation in both NumberTextStyle/resolve(_:) and PatternTextStyle/resolve(_:) because neither of them needed to actually merge the proposal for any other reason than to access Snapshot/nonvirtuals.

@oscbyspro oscbyspro added the enhancement improvements n' stuff label Aug 13, 2022
@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 13, 2022

I think that PatternTextStyle/resolve can use the same reduce function as format and interpret. At least it looked like it before Xcode 14.0 beta 5 decided to piviot its career to that of a pyromaniac. Anyways, it would be more performant and less code to maintain.


PatternTextStyle/resolve(_:with:) now uses the reduce method directly, ba6f11c.

@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 22, 2022

Fixed a bug in 5940f6b where localizable styles marked as equals(()) would not be localized on setup (input shadowed output). It’s not that note worthy, because the bug had not been introduced outside of beta.

@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 23, 2022

Removed Lock in cfa97c6 because it was made obsolete by #161

@oscbyspro
Copy link
Owner Author

Cleaned up debug messages in ac76804 as:

 [DiffableTextViews] [context]: [message]

@oscbyspro
Copy link
Owner Author

Fixed #161 suggestion + autocorrection + reentrance selection flash in cb8eb65.

@oscbyspro
Copy link
Owner Author

oscbyspro commented Aug 31, 2022

PatternTextStyle: made state public.
PatternTextStyle: added initializer parameters.
PatternTextPlaceholders: made ExpressibleByDictionaryLiteral.

c48c0b0, 657122b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brrr such code, much quality enhancement improvements n' stuff
Projects
None yet
Development

No branches or pull requests

1 participant