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

Improvements and fixes to NSTextList-related code #297

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

fjmilens3
Copy link

Fixes for some NSTextList-related issues (#293, #294, #295).

Changes include:

  • Changes to isEqual: behavior for NSTextList and NSParagraphStyle.
  • Includes a default empty array value for NSParagraphStyle _textLists (similar to _tabStops).
  • Fixes NSAttributeString's rangeOfTextList:atIndex: to avoid lockup conditions for adjacent ranges.
  • Fixes NSAttributeString's itemNumberInTextList:atIndex: to return the item number for the list.
  • Includes test cases for the above.

Notes:

  • I only installed GNUStep last week and don't have a full setup for building and working on GNUStep itself yet. I was able to build libs-gui with some opt-outs. If I keep running into problems, is the gnustep-discuss list the best place to ask for help?
  • In addition to the test cases included here, I've tested the code by including it in category methods within my existing GNUStep installation. They seem to resolve the problems that led me to report the bugs.
  • This is my first PR so if there are any general GNUStep code practices I'm missing, let me know and I'll adjust accordingly.

{
NSArray *textLists = [style textLists];
return 0;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the return value in Cocoa (I'd have expected NSNotFound but it seems to return 0 instead).

itemNumber++;
}

if (buffer[index-1] == '\r' && buffer[index] == '\n')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the use of NSCharacterSet newlineCharacterSet may be overkill, would we use '\n' internally or do we need to support Windows CR+LF combinations?

@@ -303,6 +303,8 @@ - (id) init
[_tabStops addObject: tab];
RELEASE(tab);
}

ASSIGN(_textLists, [NSArray array]);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instances in Cocoa always have an empty array set for the default value, so I included this here and below.

We weren't even saving the textLists in some of the serialization paths, and other fields like text blocks and text tables are ignored as well. I'd suggest deferring any work on that part unless we make the changes as a batch and bump the object version; I'm not that familiar with any of this yet, and I don't want the PR to get out of hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant