boxblinkracer / phpunuhi

PHPUnuhi - The easy composable framework to validate and manage translations
MIT License
72 stars 5 forks source link

Add final newline option for json #32

Closed JoshuaBehrens closed 10 months ago

JoshuaBehrens commented 11 months ago

In our projects we use .editorconfig to ensure we always have a newline at the end of most files. This tool removes this newline. This is annoying for automatic commits fixing translation files. So I though I just add such a feature as well in here.

I was not sure how to test it though. I am missing saver tests. I can't find them so I thought I just put this pull request in the wild and maybe one can show me which test I shall extend. I am also open to add this feature for more than JSON translations if I can be sure how to test this. I am also missing a PHP 7.4 installation on my machine. How do you develop in PHP 8.0?

boxblinkracer commented 11 months ago

hey @JoshuaBehrens aaa awesome, didnt think about that yet :)

so regarding tests...yeah...didnt have time to use mocks or ifaces for writers and write tests for the core-saving logic, but you can help on that if you have time haha

regarding the feature, lets do this the only question i have is,, wouldnt it be better to tightly couple it to the storage setting, so like the alphabetical order? benefit: no argument required and directly bundled with storage settings downside: every file-storage needs to reimplement it

also, can we use this name for argument or setting: eol-last its a bit shorter and lookes more like this: https://eslint.org/docs/latest/rules/eol-last

what is your opinion on these ideas?

JoshuaBehrens commented 11 months ago

I can write your first tests for the savers, no worries.

I already put it to the key sorting. Isn't it the storage setting? Or where are you in the xml?

I am open to the name change. As I was coming from .editorconfig I just copied the name. I think eol-last is reasonable and easy to find with the xsd.

boxblinkracer commented 11 months ago

hi hi

yeah sounds cool

i meant the storage xml yes <json sort=true eol-last=true

or so? something like this?

JoshuaBehrens commented 11 months ago

See my changed files. There is an example about it. I think I placed it right, didn't I?

boxblinkracer commented 11 months ago

yes, perfectly placed man :)

JoshuaBehrens commented 11 months ago

@boxblinkracer can you have a look? In my test I noticed, that the indention size reformatting seems to be broken. But I am not sure what your previous thoughts were there :D maybe you can comment on that

boxblinkracer commented 10 months ago

ah so sorry yes its brilliant man, can i help with anything, should i merge and finish the rest or do you want to do it? and what is happening with the broken reformatting?

JoshuaBehrens commented 10 months ago

I have a commit fixing that. So I'd be glad if you take a look at the new feature and the fix and merge if you like :) I have so far no further intentions to push anything on that branch

JoshuaBehrens commented 10 months ago

Ty for merging

dancing duck