search

mitoteam / jpgraph

JpGraph library composer package with PHP 8.3 support
https://packagist.org/packages/mitoteam/jpgraph
Other
33 stars 5 forks source link

Better Windows OS detection under Cygwin #11

Closed oleibman closed 2 years ago

oleibman commented 2 years ago

I stumbled across this by accident, and it isn't all that important for PhpSpreadsheet since Cygwin Php is frozen at 7.3 and PhpSpreadsheet will soon stop supporting that release. But ...

In several places in jpgraph.php, it is coded:

if (strstr( PHP_OS, 'WIN' ) {

with the intent of differentiating Windows from other systems. Unfortunately, that coding has the effect of treating Cygwin as Windows, when it should be treated as other, and we consequently fail in Cygwin. These checks might just as easily be replaced with:

if (PHP_OS_FAMILY === 'Windows') {

Now we should succeed in Windows, Cygwin, or anywhere else.

oleibman commented 2 years ago

It looks like PHP_OS_FAMILY wasn't introduced until 7.2, so, if you have to support earlier releases, you may have to do something cleverer than my suggestion.

f1mishutka commented 2 years ago

Just pushed commit ff566ee957d5ca648146c29f0ada5147001a1a43 with this issue fix.

Could you please confirm latest code in main branch works well with it now? I'll release new version after confirmation.

oleibman commented 2 years ago

Works great for Windows, Cygwin, and Linux.

f1mishutka commented 2 years ago

v10.2.4 released Thank you!