[TYPO3-core] RFC: #9574: Configuration enhancement for indexed_search

[Michael Stucki]

Hi Steffen,

I reviewed the code, there are still many things that need improvement.

indexed_search/ext_typoscript_setup.txt:

1.  plugin.tx_indexedsearch.templateFile

You cannot change the value for this! If you want to change it, you need
to use a [compatVersion] condition and write a hook for the update
wizard to mention this.

Additionally, we use to emphasize such changes with three !!! in the
ChangeLog.

2. #what to display
   # if results should be outputted on different page use the pid here

=> Please give more care to that. Always add a space after a comment
   mark, and fix the English (page => pages).

3. "use the pid here"

What pid? Needs to be more clear.

4. plugin.tx_indexedsearch.show.advancedSearchLink

Value is changed. Must be done with a compatVersion, too.

5. plugin.tx_indexedsearch.defaultResultsPerPage = 2

Can't be 2. The current default is 10 so unless there is a good reason
to change it, please leave that as default.

Also, the name "defaultResultsPerPage" sounds odd to me. Why default? If
you change this value, you also change the number of results per page,
right? If so, remove the "default" from the name.

6. ...pageBrowser.pageLinks_stdWrap.preCObject.dataWrap

I can't find a wrap in this value. Should this be "data" instead of
"dataWrap"?

7. plugin.tx_indexedsearchbox

Should be named "tx_indexedsearch_box" (separate "box" from the rest).


indexed_search/locallang.xml:

1. flexform_indexed_search.sheetTitle

Consistency: "Indexed Search" is written with capital "S" although all
other labels use lower-case.

But I think that using upper-case "S" would be correct anyways (it's
part of a title). So if you like to fix it, you can probably do so. As
far as I see, only BE labels would be affected, and their meaning
wouldn't be changed anyway. So it should be ok if you leave your
upper-case label and fix all others instead...


indexed_search/pi/class.tx_indexedsearch.php:

1.
+205 foreach ($this->code as $c) {
+206  switch (t3lib_div::strtolower($c)) {
+207   case 'form':
+208    if ($this->conf['code'] != 'form' && !$this->conf['showForm']) {
+209     break;
+210    }

I don't understand why the code must be "form" here. If it has a good
reason, please add a comment to it.

2.
+213 case 'rules':
+214  if ($this->conf['showRules']) {

I don't like to set "code = rules" and then rules are only displayed if
conf.showRules is set.

Suggestion:
- Change the default code to "defaults"
- Consider value of conf.showRules as long as the value is "defaults"
- Ignore conf.showRules as soon as the "code" is different to that

(applies to all other cases of this statement, too)

3. // Browsing nav, bottom.

The line is repeated now. Most likely a typo?

4. $pid = $this->conf['resultPid'] ? $this->conf['resultPid'] :
                                                $GLOBALS['TSFE']->id;

Remove $pid and set $this->conf['resultPid'] in ->initialize(), just
like the rest.


General notes:

1. I did not yet check the functionality of your patch. Will do so after
   you have fixed all points mentioned above.

2. I'm not quite happy that the template is changed, although I see the
   need for it. In case someone has customized the template file and
   points to that, the display will be broken after the update. I think
   this should be mentioned somewhere, with a screenshot of the part
   that must be added to that customized template.
   Again I propose to use the update wizard for that.

3. Documentation is completely missing!!

Since there is too much stuff that needs to be fixed, I am -1 to the change.

However, I must say I like the idea of your change and hope to see this
in 4.3.

- michael

> now i created a new patch where i add some missing:
> 
> * whatis is now a marker too
> * whatis has stdWrap, also the markup (this is the searchword in the
> results)
> 
> I also added missing sections / marker in the table-based template.
> 
> I use this patch now in the second client project and i'm very happy
> with it. It allows to modify output in any way to get it like designers
> want.
> The biggest advantages are:
> 
> * all is in template now
> * all can be formatted with stdWrap
> * 2 plugins (no need of macina anymore)
> * output can be splitted as indexed_serch comes with flexform
> 
> I really would like to get this in, so please take a look and test it.
> 
> vg Steffen
> 


-- 
Use a newsreader! Check out
http://typo3.org/community/mailing-lists/use-a-news-reader/