search

nette / tracy

😎 Tracy: the addictive tool to ease debugging PHP code for cool developers. Friendly design, logging, profiler, advanced features like debugging AJAX calls or CLI support. You will love it.
https://tracy.nette.org
Other
1.76k stars 218 forks source link

Dumper: added ext-ds support #484

Closed enumag closed 3 years ago

enumag commented 3 years ago

Closes #472

enumag commented 3 years ago

@dg The failure is not caused by my changes.

dg commented 3 years ago

Can you add a test?

enumag commented 3 years ago

The test would of course require ext-ds to be installed. I'm not sure how to add it on your GitHub Actions setup.

dg commented 3 years ago

In this way: https://github.com/nette/latte/blob/master/.github/workflows/tests.yml#L20

(Probably extensions: ds)

enumag commented 3 years ago

@dg How do I run the tests locally? I tried composer tester but it fails with

   Error: Interface 'JsonSerializable' not found

I have no idea why. I'm using json extension every day and in my code it always exists. So I'm very confused. Can you fix composer tester so that it would work?

milo commented 3 years ago

vendor/bin/tester -C tests should do the job

enumag commented 3 years ago

@milo thx for the advice, I fixed composer.json to use this option too.

dg commented 3 years ago

ad JsonSerializable: what operating system are you using?

enumag commented 3 years ago

@dg Ubuntu but I don't think it's important. The issue is that nette/tester isn't using the system's default php.ini without the -C option so it didn't load any extensions. It is very counter-intuitive for someone not experienced with nette/tester.

If there are reasons to not load the default php.ini in some scenarios then in my opinion it should work the other way around - use system's php.ini by default with an option to turn it off.

dg commented 3 years ago

The point is, it works for me on Windows and I had no idea there is such a problem.

This has to be solved with a custom php.ini, I'll fix it so please don't change the composer.json.

enumag commented 3 years ago

Alright, I removed the composer.json commit. Can you review the feature I'm adding? :-)

dg commented 3 years ago

Looks good, thanks