search

contao / manager-bundle

[READ-ONLY] Contao Manager Bundle
GNU Lesser General Public License v3.0
17 stars 10 forks source link

Add error when cli args aren't set properly #50

Closed ynfinite closed 6 years ago

ynfinite commented 6 years ago

Some providers set the php.ini argument register_argc_argv to 'Off'. This results in the contao-console command always showing the help page. A small error message when the $argsv / $argsc variables aren't set helps to solve this problem much faster. When register_argc_argv is set to 'On' no arguments still means that the variables are set so the help page is shown properly.

ynfinite commented 6 years ago

@leofeyer Whats the problem? Can I fix something?

leofeyer commented 6 years ago

Please be patient. @aschempp will get back to you.

aschempp commented 6 years ago

That's a strange setup and I'm not sure how to handle this. First of all, the issue would be in any Symfony console, so a generic error message would make more sense. Or maybe there is none for a reason? Also, if we check for that, there would probably be a ton of other server misconfigurations we could check and warn about.

I also imagine this error message would appear if you run the console without any argument, which is a very valid use case?

leofeyer commented 6 years ago

I think we should make register_argc_argv=On a requirement and add a check to the self-tests.

ynfinite commented 6 years ago

@aschempp I am a big fan of error messages which help the user to understand what causes the problem and tell them how to fix it. React JS is a good example for such a system. There are a bunch of error messages which actually tell you where your error is so debugging is quite fast.

The error message is not appearing when the console is called without arguments because the server args are always set even when no arguments were actually supplied. (At least on the systems I tested it)

@leofeyer I think you got a point there. Making it a requirement would solve the issue too.

aschempp commented 6 years ago

I am a big fan of error messages which help the user to understand what causes the problem and tell them how to fix it.

Me too. I don't dislike the message, I don't yet like the place it is implemented. If that issue actually exists, it's an issue in the Symfony Input component, and the message should idealy be triggered there. Because there are probably 100 settings we could check, and we can't put them all in the console script.

leofeyer commented 6 years ago

So can the ticket be closed then?

ynfinite commented 6 years ago

Yes it can be closed. Your solution is pretty good.