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

Better diff support for Grid Layouts with CDATA and preserved line feeds for some PreValues #128

Closed
davidtwamley opened this issue Apr 12, 2017 · 5 comments
Assignees

Comments

@davidtwamley
Copy link

We've encountered an issue several times where the json for a grid layout is modified by multiple developers (even simple changes like which grid editors are allowed) on their feature branches and then merging them is difficult because the json is collapsed down to a single line. In these cases we've resorted to extracting out the raw json from both and putting them in beyond compare. It would be very nice though if instead, these values were wrapped in a CDATA and line feeds were preserved.

Here is an abbreviated example of what we see:

<DataType Name="Grid - Text Page" Key="" Id="Umbraco.Grid" DatabaseType="Ntext">
  <PreValues>
    <PreValue Id="1134" Value="{&#xD;&#xA;  &quot;styles&quot;: [&#xD;&#xA;    {&#xD;&#xA;      &quot;label&quot;: &quot;Background Image&quot;,&#xD;&#xA;      &quot;description&quot;
    ...58,000+ chars..." Alias="items" />
  </PreValues>
</DataType>

We are using LeBlender but I don't think that matters. I believe storing json in Ntext is becoming more popular too so maybe a generic fix would be best. Some suggestions:

  • Always use CDATA for PreValue so the line feeds are preserved
  • Detect when CDATA for PreValue would be better, maybe any of these:
    • First char is {
    • Value contains \n
    • Character length > 300

Thanks!

@davidtwamley
Copy link
Author

Looks like KevinJump/uSync#28 is one approach for achieving this.

@KevinJump
Copy link
Owner

yeah - the mappers for the grid (especially le blender) got quite a bit of a rewrite recently, so i need to move the pre-value stuff into the core so its used in the right places. but yet that PR does show the solution. :)

@KevinJump KevinJump self-assigned this Apr 13, 2017
@KevinJump KevinJump added this to the 3.3.3 - Patch Release milestone Apr 13, 2017
@KevinJump KevinJump modified the milestones: 3.4.0 Update Release , 3.3.3 - Patch Release Apr 24, 2017
@KevinJump
Copy link
Owner

will have to bump this to a version change release - as it will change the format of the export file.

KevinJump added a commit that referenced this issue May 20, 2017
moves it from attribute Value , also has legacy support so old syncs
where value is an attribute will still work (but a new sync is always
recommended on a version bump)
@KevinJump
Copy link
Owner

IN next version (baring testing)

@KevinJump KevinJump modified the milestones: 3.3.4 - Patch Release , 3.4.0 Update Release May 20, 2017
@davidtwamley
Copy link
Author

Very nice! Thank you very much 👍

marcemarc pushed a commit to marcemarc/uSync-Legacy that referenced this issue Aug 4, 2023
marcemarc pushed a commit to marcemarc/uSync-Legacy that referenced this issue Aug 4, 2023
Attempt to work out if the container name is being used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants