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

Replace NSString everywhere #469

Open
ktraunmueller opened this issue May 9, 2019 · 0 comments

Comments

1 participant
@ktraunmueller
Copy link
Owner

commented May 9, 2019

With the recent improvements to Swift's String type (UTF8 storage, isAscii performance flag, etc), it seems about time to replace all uses of NSString and NSMutableString.

This could potentially become a huge refactoring, as NSString and NSRange are everywhere. Maybe we should introduce a custom type ASCIIString (backed by a String or gasp! maybe even a ContiguousArray<CChar>?) that provides Int- and NSRange-based indexing and slicing similar to NSString (but assuming ASCII-only strings). This way, most of the existing code wouldn't need to change.

Note: for any code dealing with Unicode text (e.g., input transformation code), expect subtle issues because of differences how NSString and String report the length of a string: NSString.length is the number of UTF-16 code units, while String.length returns the number of perceived characters.

Example (insertedText is an NSMutableString):

(lldb) po insertedText
À
(lldb) po insertedText.length
2

This shouldn't be a problem in most of the code, though, because it's ASCII except for the input stage.

Performance-related: be careful / verify if SourceModel's texSource is referenced only by SourceModel, to prevent triggering creating copies (CoW) when modifying the string.

Prerequisites: #468

@ktraunmueller ktraunmueller added this to the Internals milestone May 9, 2019

@ktraunmueller ktraunmueller modified the milestones: Internals, 1.11 May 23, 2019

@ktraunmueller ktraunmueller changed the title Replace NSString with String everywhere Replace NSString everywhere May 23, 2019

@ktraunmueller ktraunmueller self-assigned this May 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.