Enabling and disabling Search API is useful when running with multiple development and production environments.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2478349-13-enabled.patch | 1.05 KB | joelpittet |
Enabling and disabling Search API is useful when running with multiple development and production environments.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2478349-13-enabled.patch | 1.05 KB | joelpittet |
Comments
Comment #1
codesidekick commentedThis patch adds the enabled property.
What would be better is if a list of excluded parameters (module, id and name) were provided and a warning was shown to the user if they tried to override one of those protected variables. All other variables could then override. In the end because this is environment configuration developers should be given flexibility rather than limited to what they can and can't override.
The function that I patched contained a fair bit of romantic commenting "Using 'we' throughout instead of explaining what is being done in an absolute form.
An example would be
Is a bit romantic whereas
Is more absolute, and although a bit dry arguably more professional (though it comes down to personal taste).
In this patch I didn't use romantic commenting but feel free to change the comment I added if you want it to conform to the rest of the function.
Comment #2
grayle commentedI noticed there was a request for the enabled setting as well as one for the class, so I made a patch which allows everything to be overridden.
Comment #3
nicksanta commentedPatch from #2 works.
Comment #4
damienmckennaFYI you should only set the "assigned" value to your name if you're continuing to work on an issue, if you've finished your proposed changes then you should leave it unassigned.
Comment #5
joelpittetCan this be done without PHP needing to be updated? aka without
array_replace_recursive?Comment #6
joelpittet#1, does enough to cover this, though needs a comments like provided in #2. The code in #2 may be overstepping the scope of this issue a bit, though it is nice, it would be better if it didn't add the extra
array_replace_recursive, and the keywordfncis a bit cryptic.Comment #7
joelpittetThis is just the patch from #1 with the documented param.
Comment #8
grayle commentedReroll of #2, haven't been able to test it though. Not sure it's needed anymore either, I think the current dev version + #7 already has everything except machine_name? It's also, well, too much code for what needs to happen. Yes, you can easily add support for another array like options but the chances that search_api is going to add something like that now is slim to none. I just like writing code and I had some time, so it's here if you want to give it a shot.
Comment #9
vuilThe patch of #8 needs to be re-rolled.
Comment #10
solideogloria commentedPatch #8 uses a remake of
array_replace_recursive()with the motivation that D7 supported PHP < 5.3 so it might not exist. This is no longer the case, since D7 only supports PHP 5.3 and up, so I would think that the patch could be made simpler by usingarray_replace_recursive()now.Comment #11
solideogloria commented@vuil I don't think #8 should be used. Those changes to filter the parameters should be a separate issue.
#7 does what the issue "says on the box".
Comment #12
vuilOK, Thanks @solideogloria! I will review it.
Comment #13
joelpittetRerolled #7
Comment #15
vuilComment #17
bjaxelsen commentedI think there is a problem with the recent commit.
It should be
$component->enabledrather than
$server->enabledComment #18
solideogloria commented@bjaxelsen #3109309: TypeError and Undefined variable due to merge conflicts.
The fix still needs to be committed and it's been a while. Maybe someone could ping a maintainer?
Comment #19
solideogloria commentedAlso, this issue should probably be reopened until it's fixed, but only maintainers can reopen.
Comment #20
vuilI set the issue back to Needs work.
Comment #21
solideogloria commented