Problems with NSTextList and NSParagraphStyle isEqual: implementations
Opened this issue · 3 comments
The implementation of isEqual: for NSTextList and NSParagraphStyle needs to be reconsidered. As a practical matter it causes problems with list handling in NSTextViews/NSTextStorage.
(I'd also be happy to submit some PRs on these if nobody else is interested, but at this time, I've just started playing with GNUStep and don't have it building locally. The three issues I'm reporting are all interrelated to some degree in terms of their effects.)
NSTextList
For NSTextList on GNUStep, we do the following and compare some of the member fields:
Lines 54 to 67 in fb61a0f
On the other hand, Cocoa itself seems to treat each NSTextList as unique, even if they have the same values:
NSTextList *list1 = [[NSTextList alloc] initWithMarkerFormat: @"{box}" options:0];
NSTextList *list2 = [[NSTextList alloc] initWithMarkerFormat: @"{box}" options:0];
NSLog(@"Equals: %d", [list1 isEqual: list2]);
On Cocoa this prints 0 but on GNUStep it prints 1. I suspect the Cocoa implementation's isEqual: only looks to see if they're the exact same object, which makes sense. Two lists might have the same values (marker format, starting value) but they would still be fundamentally different lists. (This also relates to the below topic.)
NSParagraphStyle
For NSParagraphStyle it currently doesn't include the textLists as part of the equality test:
libs-gui/Source/NSParagraphStyle.m
Lines 597 to 625 in fb61a0f
That appears to be causing a problem where NSParagraphStyle runs in attributed strings get merged together when they shouldn't. In this case it can happen when adjacent paragraph styles differing only by lists end up getting combined when they shouldn't, but it could conceivably happen in other situations as well (text blocks perhaps?).
Note that adding the textLists to the comparison here won't have any effect unless the NSTextList comparison itself is fixed to treat each instance as unique.
To see what I mean about it causing paragraph styles to be merged together when they shouldn't, see the following:
- (void) paragraphMergeExample
{
// Create two lists (even with different parameters for emphasis)
NSTextList *list1 = [[NSTextList alloc] initWithMarkerFormat: @"{box}" options: 0];
NSTextList *list2 = [[NSTextList alloc] initWithMarkerFormat: @"{decimal}" options: 0];
// Create two paragraph styles
NSMutableParagraphStyle *style1 = [[NSParagraphStyle defaultParagraphStyle] mutableCopy];
NSMutableParagraphStyle *style2 = [[NSParagraphStyle defaultParagraphStyle] mutableCopy];
// Change the paragraph styles so that they don't get merged together (comment/uncomment as needed)
/*
[style1 setHeadIndent: 36.0];
[style2 setHeadIndent: 72.0];
*/
// Use one list per paragraph style
[style1 setTextLists: [NSArray arrayWithObject: list1]];
[style2 setTextLists: [NSArray arrayWithObject: list2]];
// Create an attributed string for each list
NSAttributedString *str1 = [[NSAttributedString alloc]
initWithString: [NSString stringWithFormat: @"%@ list1\n", [list1 markerForItemNumber: 1]]
attributes: [NSDictionary dictionaryWithObject: style1 forKey: NSParagraphStyleAttributeName]];
NSAttributedString *str2 = [[NSAttributedString alloc]
initWithString: [NSString stringWithFormat: @"%@ list2\n", [list2 markerForItemNumber: 1]]
attributes: [NSDictionary dictionaryWithObject: style2 forKey: NSParagraphStyleAttributeName]];
// Append the attributed strings to the text storage
NSTextStorage *textStorage = [textView textStorage];
[textStorage appendAttributedString: str1];
[textStorage appendAttributedString: str2];
// Compare the styles we actually get back
NSParagraphStyle *answer1 = [textStorage attribute: NSParagraphStyleAttributeName
atIndex: 0 effectiveRange: NULL];
NSParagraphStyle *answer2 = [textStorage attribute: NSParagraphStyleAttributeName
atIndex: [textStorage length] - 1 effectiveRange: NULL];
// The two paragraph styles shouldn't be the same, but they will be (under current GNUStep)
NSLog(@"Original styles: %@, %@", style1, style2);
NSLog(@"Returned styles: %@, %@", answer1, answer2);
}
In the log output:
2024-09-23 16:25:43.507 HelloApp[3347:3347] Original styles: <NSMutableParagraphStyle: 0x556ea83960c0> Alignment: 4 LineSpacing: 0.000000 ParagraphSpacing: 0.000000 LineBreakMode: 0, <NSMutableParagraphStyle: 0x556ea83acaa0> Alignment: 4 LineSpacing: 0.000000 ParagraphSpacing: 0.000000 LineBreakMode: 0
2024-09-23 16:25:43.507 HelloApp[3347:3347] Returned styles: <NSMutableParagraphStyle: 0x556ea83960c0> Alignment: 4 LineSpacing: 0.000000 ParagraphSpacing: 0.000000 LineBreakMode: 0, <NSMutableParagraphStyle: 0x556ea83960c0> Alignment: 4 LineSpacing: 0.000000 ParagraphSpacing: 0.000000 LineBreakMode: 0
Note that we put in two paragraph styles with different list items (0x556ea83960c0 and 0x556ea83acaa0), but only get the one paragraph style instance back (0x556ea83960c0) because of the incorrect merging.
If you uncomment the section in the example, this won't happen because the headIndents will be different on each. Something similar should probably be happening when the textLists (and other similar fields) are different as well.
Thank you for these great observations. The problem here is that nobody else seems to be using NSTextList at the moment on GNUstep. Otherwise these issues would have been noticed before. It would be great if you could contribute fixes. The best way is to provide a pull request on GitHub and it would even be better if that included a test for the functionality.
@fredkiefer, I'll try to get libs-gui building locally and see if I can send a PR over for this and the other two issues (#293, #294, #295). I hope to have a chance to work on it soon.
I've actually been quite impressed with GNUStep so far (it reminds me of how much I enjoyed programming in Cocoa on Mac OS 10.4 many years ago). If nobody's been using it for applications that are heavy on text editing, I doubt anyone would have found the issue.