patsplat / plist

All-purpose Property List manipulation library
http://www.rubydoc.info/gems/plist
MIT License
306 stars 71 forks source link

Multiline string values must not be indented #31

Closed bkoell closed 3 years ago

bkoell commented 7 years ago

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 commented 7 years ago

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

patsplat commented 7 years ago

Please provide a test for the indention bug.

mattbrictson commented 7 years ago

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 commented 3 years ago

This issue is also addressed in a previous ticket: https://github.com/patsplat/plist/issues/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 commented 3 years ago

@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 commented 3 years ago

Please see https://github.com/patsplat/plist/pull/54

tboyko commented 3 years ago

@mattbrictson Would you be willing to review and accept https://github.com/patsplat/plist/pull/54 ?

mattbrictson commented 3 years ago

Closed in favor of #54