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

Multiline string values must not be indented #31

Closed
wants to merge 12 commits into from

Conversation

bkoell
Copy link

@bkoell bkoell commented Jan 24, 2017

Hey everyone,
multiline plist values get modified even when you don't do any changes. Multiline strings are expected to be not indented or the indentation will be part of the string that is read later on. You can easily see this behaviour creating Plists with Xcode for example.

I ran into this especially because we are using multiline markdown values which then render incorrectly because of indentation changes.

Would very much appreciate if this could make its way into the gem. Don't want to deploy a custom one all the time.

Thanks from Berlin!

@bkoell
Copy link
Author

bkoell commented Jan 30, 2017

Fixed version can be installed now (does not pretty print the plist):
gem install plist.newline

@patsplat
Copy link
Owner

Please provide a test for the indention bug.

@mattbrictson
Copy link
Collaborator

Hi, I'd like to understand this PR better because this seems like a legitimate bug to me. If you are still interested in contributing, could you rebase on the latest master and add a test? I'll do my best to give you constructive feedback. Thanks!

@tboyko
Copy link
Contributor

tboyko commented Nov 24, 2020

This issue is also addressed in a previous ticket: #14

In it, @cristianbica provides a useful workaround of applying .gsub(/([^>]\n)\t+/,'\1') to the output of Plist::Emit.dump. The problem with this approach, however, is that it removes all leading tabs for multiline content. If, for instance, a string has an intentional single tab indentation, and then plist adds two tabs automatically, this will remove all three tabs instead of the desired behavior of just two.

@mattbrictson : you mention in ticket #14 that the indentation behavior is a bug, but I do not see at attached PR to fix it. Do you know if anything became of this?

@mattbrictson
Copy link
Collaborator

@tboyko sorry, I don't know what became of this bug. To be honest I have not been tracking this repo since I last commented back in 2017.

Reviewing #14, it very clearly is a bug in my opinion because if you save a plist containing \n and then read that same plist back in, the string has changed. So data is getting corrupted. Saving and loading should not cause data to change.

Again, I am not super familiar with this repo these days but I would think that it should be possible to add a failing test for this scenario and the implement a fix, maybe based on what @bkoell originally proposed here. I'm not aware of any other attempts to fix it. Do you want to give it a shot?

@tboyko
Copy link
Contributor

tboyko commented Nov 30, 2020

Please see #54

@tboyko
Copy link
Contributor

tboyko commented Dec 8, 2020

@mattbrictson Would you be willing to review and accept #54 ?

@mattbrictson
Copy link
Collaborator

Closed in favor of #54

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.

None yet