Issues with the settings file and a proposition

[kudashevs]

@freekmurze Hello,

I’ve just started to use Laravel 9 and it turn out, that I cannot save ~/.ignition.json not in Windows environment (it doesn’t find the HOMEDRIVE and HOMEPATH in the environment), nor in Linux environment (it uses the HOME directory of the PHP process, so it fails due to lack of permissions). And it seems, that it not only my problem (something similar happens here #34).

I have a proposition to solve this problem by adding a possibility to provide a path of the settings file. This proposition includes two steps:

• update ignition component with the possibility to provide the specific path to the IngnitionConfig
• update laravel-ignition to use this new ignition possibility under the hood (and keep the settings file path in the component config)

Frankly speaking, I’ve almost finished the first step. This is a new component FileConfigManager in the src/Config folder which is implemented through a new ConfigManager interface. The interface is a little bit over-engineered with the getSource method, however, I've found it useful for testing purposes and it could be useful in the future (but it could be removed, though). So, the new FileConfigManager gets all the responsibility to manage the settings file:

• it accepts a path through the constructor and validates it
• if the path is not provided it tries to find an appropriate path from the environment (was taken from DefaultConfigFinder)
• it can load the options from a file
• it can save the options to the file
• it can create the file (and this seems the most important because DefaultConfigFinder didn't do that, so DefaultConfigFinderTest didn't work properly for me if I didn't have a created settings file)
• it is covered with tests in some weak points

Could you please give it a look and make a review?
Btw, I have some questions about the realization:

• am I allowed to throw a specific Exception in case, if the provided path is incorrect?
• is it ok, to use \Trowable from the global scope, or you prefer to import it explicitly?

So, if it looks appropriate to you, give me please some feedback. I would like to polish the first step and then implement the second step of the proposition.

[AlexVanderbist]

Hi @kudashevs! First of all, thanks for taking an in depth look at this issue and coming up with a seemingly great solution at the same time. I'll look at your PR on spatie/ignition next and provide some feedback as part of a PR review.

To quickly answer your thoughts:

am I allowed to throw a specific Exception in case, if the provided path is incorrect?

If at all possible, I'd like to avoid throwing exceptions inside of Ignition. They'll be caught by Laravel's exception handler and rendered using Symfony's exception renderer as a fallback. Not only does this look bad (Ignition failing and Symfony taking over) but it'll also cover up the actual exception that was occurring.

Specifically for an incorrect config path I'd love it to fail gracefully, maybe try the default path instead. We could then provide feedback to the developer using Ignition's health check endpoint. Currently there's only a check for running solutions but I think it'd be nice to clean up that controller and provide a valid_config_path and maybe even a config_path_writeable key in that response.

is it ok, to use \Trowable from the global scope, or you prefer to import it explicitly?

No specific preference on our end. I'd suggest we use Laravel's code style and import it explicitely?

Thanks again for your input and great PR, I'm going to look into it right now :)

[kudashevs]

Hello @AlexVanderbist

Thank you very much for the review. It is thorough and has useful suggestions, I appreciate it.

P.S. I'll come up with the final PR in a couple of days.