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

Implement NSArray descriptionWithLocale* methods #2256

Merged
merged 2 commits into from
Mar 20, 2017

Conversation

aballway
Copy link
Contributor

Fixes #2073

return StubReturn();
thread_local unsigned int indent = 0;
NSMutableString* s = [NSMutableString string];
NSString* indentStr = (level == 0) ? @" " : @"\t";

Choose a reason for hiding this comment

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

I'm torn on how this is supposed to work. Shouldn't it call the next one with the next level, and always make sure to indent itself to this level? The thread local becomes superfluous, and it's initialized with the wrong value anyway.

Copy link
Contributor Author

@aballway aballway Mar 16, 2017

Choose a reason for hiding this comment

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

https://developer.apple.com/reference/foundation/nsarray/1416257-descriptionwithlocale

UPDATE: this is possibly the most misleading documentation I have read 👎

…ntation was still rather confusing but despite the previous failure this changes the [NSArray descriptionWithLocale:indent:] to propertly use indent to propogate the indentation level which will be useable (once implemented) in NSDictionary, NSSet, etc.
++level;
auto deferPop = wil::ScopeExit([&level]() { --level; });
for (id val in self) {
for (unsigned int i = 0; i < level; ++i) {

Choose a reason for hiding this comment

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

I'm still not sure this is totally correct; if you nest arrays, you'll double-indent them because you're appending level*"spaces", and then passing level to the nested value which will prepend level*"spaces"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be expected behavior, our test for NSArray description depth still passes w/ this change

Choose a reason for hiding this comment

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

:(

i guess we should file a task to do this for dictionary too.

@aballway aballway requested a review from ms-jihua March 17, 2017 22:12
@aballway aballway merged commit d1072cf into microsoft:develop Mar 20, 2017
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.

4 participants