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

RFC: Encode/Decode Data Lossless (Format-preserving Printing) #52

Open
f3l1x opened this issue Jun 2, 2020 · 8 comments
Open

RFC: Encode/Decode Data Lossless (Format-preserving Printing) #52

f3l1x opened this issue Jun 2, 2020 · 8 comments

Comments

@f3l1x
Copy link
Member

f3l1x commented Jun 2, 2020

Prologue

This RFC aims to keep original NEON format and prevent data losing. It's in correlation to #51.

Picture you have really big NEON file holding big definition schema with comments. It's very hand to have this kind of definition schema, because people keep their comments there and after all this NEON file is converted to JSON.

At this time we handle it like this:

  • There is table in DB holding NEON column and JSON column.
  • Someone update NEON and both columns get updated.

So far so good.

Unless you need to apply some automatic migrations, in current situation you can't. We'd loose user comments and they are really needed.

Current API

foo.neon

# List of users
users: [1,2,3,4]
# Revision v0.0.1

test.php

$content = file_get_contents('./foo.neon');

$data = Nette\Neon\Neon::decode($content);
$data->users[] = 5;

file_put_contents('./foo.neon', Nette\Neon\Neon::encode($data));

foo.neon

users: [1,2,3,4,5]

I totally understand how this API works and it works great for encoding/decoding, but it does not prevent original content.

Proposed API

AST

I am not sure how this API should look like. Maybe it would be needed to create some kind of AST parser and understand comments properly.

AST parsers is maybe too heavy and someone would bring up simpler solution.

Context Merging

Method Neon::encode does not have any context of original file, passing original file instead of string could be the way. I am not sure.

$data = Nette\Neon\Neon::merge('./foo.neon', ['users' => [5]);
file_put_contents('./foo.neon', $data);

It's an idea of adding extra feature to NEON. Maybe someone think the same way. Thanks for a feedback.

@mabar
Copy link
Contributor

mabar commented Jun 2, 2020

Conceptually HHAST works like that - Hack lang AST processor which preserves comments and whitespace https://github.com/hhvm/hhast

@ondrejmirtes
Copy link

This is called format-preserving printing, and for example nikic/php-parser has that, so we could draw inspiration from there :)

I personally would love this feature for PHPStan, but it's outside of my wheelhouse as I don't understand very well how parsers work (but I have expertise in working with the resulting AST :)).

@f3l1x f3l1x changed the title RFC: Encode/Decode Data Lossless RFC: Encode/Decode Data Lossless (Format-preserving Printing) Aug 13, 2020
@dg
Copy link
Member

dg commented Oct 13, 2021

I've added the AST parser to Neon 0fac117, but preserving the original formatting is pěkný omrd.

I've created an experiment that partially works, but to perfection is a long way. If I finish this, I deserve a really big beer from you :)

@ondrejmirtes
Copy link

Awesome! PHP-Parser does a great job at this, perhaps you can look at the implementation and get some inspiration out of that: https://github.com/nikic/PHP-Parser/blob/master/doc/component/Pretty_printing.markdown#formatting-preserving-pretty-printing (but it's pretty complex)

@dg
Copy link
Member

dg commented Oct 13, 2021

Yes, its too complex a přitom taková blbost.

@f3l1x
Copy link
Member Author

f3l1x commented Oct 15, 2021

Deal. Beer and vindaloo included ;-)

@dg
Copy link
Member

dg commented Oct 18, 2021

Such an experiment https://ne-on.org/?diff

@dg
Copy link
Member

dg commented Oct 20, 2021

I released version 3.3.0, which has a newly written AST parser from scratch. And also a renderer from AST.

The Updater tool is in https://github.com/nette/neon/tree/format-preserve. I need some cooperation here. I don't have a use for this feature, so I can neither test it properly nor design it well.

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

No branches or pull requests

4 participants