-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
toon export
#19991
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
base: master
Are you sure you want to change the base?
toon export
#19991
Conversation
liviuconcioiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Also, you should update the code, since #19957 has been merged.
A few things:
Can you add this so it will be possible to change the separator and indent.
$leaf = new TextPropertyItem(
'separator',
__('Columns separated with:'),
);
$generalOptions->addProperty($leaf);
$leaf = new TextPropertyItem(
'indent',
__('Indent'),
);
$generalOptions->addProperty($leaf);You should also make changes in src\Config\Settings\Export.php so the values can be seen when exporting and also be able to use them from config file
$cfg['Export']['toon_separator'] = '|';
$cfg['Export']['toon_indent'] = 4;Also, can you add a small text to documentation import_export.rst - #export?
Thank you!
5947131 to
5a4add9
Compare
|
@liviuconcioiu @williamdes |
ce5b4eb to
c79b0ce
Compare
liviuconcioiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@faissaloux I left some comments.
- Add support for tab as delimiter - https://toonformat.dev/playground.html
- Renamed from Toon to TOON
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the documentation review comments, it looks okay
Thank you !
Edit: rebase all your work please
25ad49f to
440c403
Compare
|
Documentation updated, code rebased. |
williamdes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good for me, this is ready to merge I think
I will let @kamil-tekiela review one last time
| public function exportRawQuery(string|null $db, string $sqlQuery): void | ||
| { | ||
| if ($db !== null) { | ||
| DatabaseInterface::getInstance()->selectDb($db); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is two calls to getInstance that are deprecated, at first glance I have no idea of what we migrated to.
But other files should say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I have checked for this but found that all other classes uses the same deprecated method. I have found that it's ignored so I'll ignore it for this file too.
Should be fixed in another PR.
7ada6d5 to
4bf2943
Compare
kamil-tekiela
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code quality-wise it looks ok. It's mostly copied and adapted from other plugins. I did not verify the logic or adherence to spec.
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
d5a1dfd to
6faaca1
Compare
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Signed-off-by: faissaloux <fwahabali@gmail.com>
Resolves #19945