Feature: Add bootstrap hook

[Peterragheb]

I think it'll be useful to have a beforeGenerating callback/hook.
It can be used to replace a service container binding or fake certain events for example.

I think it's a better option instead of listening to the default Laravel CommandStarting event and checking on the command type. Especially since the package already provides an afterGenerating callback.

[shalvah]

I'm not against this, but my concerns:

1. Wouldn't the hook be more useful with some arguments (such as the current config)?
2. Is there a specific reason for running it before the bootstrap() phase?

The point of hooks in Scribe is to not just tell you when something is happening, but give you tools to adjust it. As it stands, this is just a case of "run this code before you run the actual command", which doesn't seem like such a compelling use case to me. You can easily do that by the ways you mentioned or by creating your own subclass of the command.

A third concern is the name. The generating in afterGenerating refers to the HTML/Blade output phase (ie in contrast to the "extracting" phase). So I would read a beforeGenerating hook as being run just before that happens. I'd probably go with onStart or something similar, but again, this brings me to the previous points (why).

[shalvah]

Oops, big fingers closed by mistake.

If the argument is that it's more convenient to add some code in your service provider than to create a new command, I can understand that.😄

In which case, I'd accept this with a few changes:

• rename the hook, maybe to Scribe::bootstrap()
• move it into the bootstrap() method, after Scribe has done its own bootstrap
• pass it useful arguments, so it can inspect Scribe's state. I'd say probably the config instance and command instance.

[Peterragheb]

Hey thanks for taking the time to review the PR 😃

I understand your concerns and I'll do the following:

1. Move the callback call to the end of the bootstrap method
2. Pass the command instance to the callback (I think it's redundant to pass the config since it's accessible from the command object instead I think adding a setter is better, what do you think?)
3. About the naming of the hook, I understand your point. But I'm not sure about naming it bootstrap or onStart since they are very generic and are not specific to the GenerateDocumentation command especially since the package has multiple commands. What do you think about naming it beforeGenerateCommandStarts?
   Or maybe move the callback to be attached to the command instead so we can set it using
   GenerateDocumentation::onStart()

[shalvah]

maybe move the callback to be attached to the command instead so we can set it using
GenerateDocumentation::onStart()

This isn't a bad idea, but since we've been exposing APIs via a single class, Scribe, I'd like to keep it that way.

Your concern about the generic name is correct, but the multiple commands are really just "support" commands. They don't have a lot of logic and don't really need any user tinkering. So I think bootstrap() is okay.

I think adding a setter is better, what do you think

Not sure what exactly a setter would be for here (modifying the config? I'm not sure I want to encourage that pattern.). Either way, yeah, let's just keep it simple and pass the command instance.

[Peterragheb]

Sounds good 👍 I've updated the PR

[shalvah]

Can you send a docs PR as well? This is the page: https://scribe.knuckles.wtf/laravel/hooks. You can probably just click edit there.

[Peterragheb]

@shalvah I created a PR on the docs repo.
Will you be creating a new release tag soon with the new changes?

[shalvah]

Released 4.10.0