TYPO3 Core

Well, I had a look at this. I think your solution should work in general but I don't like to add another SQL statement only for counting. Let's think about a more elegant solution.

BTW, one word about patching. Please be more exactly when working with patches. With this patch it is very hard to find the actual changes because tabs have been removed, blank lines added, etc. Please try to make the patches as small as possible. Thanks.

Regards, michael

Hello Michael,

What is so wrong about my patch files? I dont understand! I use cygwin and diff with u parameter, thats all. But I admit that I have formated the code like I wanted because I really don't like the formating of the IS code at all. But I did this because I needed this code in another extensions as well. But its all there lines, +/ , version...and its small too, ... don't understand.

Anyway, the first patch was stupid, I agree. Please take a look at the next patch its far better :).

Greets,

Chi

Yes, this one is definitely better. If you can't see what I mean by simply opening the 1st diff, then I'm sorry I can't tell you more.

Anyway, thanks for the patch. I will test it and get back to you.

Regards, michael

Hello Michael,

I have found another major bug with pagecounting in function getResultRows:

var show_resume is ignored when putting together resultRows. this is really bad because pagecounting and resultRows gets totally confused in the further computing.

solution: see final patch (some fixes to getResultRows() )

btw the better patch was buggy to, anyway you dont need it anymore.

greets,

Chi

It seems you didn't understand what I tried to explain in my 1st mail. Compare your last patch and the one I just submitted. It contains exactly the same changes, but your one is 4x bigger just because it contains a lot of changes which are not changes actually (changed indentions, spaces instead of tabs, etc.). Please try to improve next time.

Anyway, the change itself looks good and luckily I have a site with exactly this problem, so I'm going to test it now and get back with a feedback as soon as possible.

Thanks! michael

Hello Micheal,

I have understood your problem already in your 1st mail. My concern is that the original code is not very good formated! Don't you agree that my patch is better formated? And thus better readable? And if you use patch it goes straightly into the is code!!! And I reuse this patch (or code piece) in my extensions (ch_is_templates), where I absolutely don't accept code formated like the original one.

BTW what kind of flags do you use to diff? Your diff has the changes inline and my diff listen them separately... It would be kind if you told me this so I can improve my technique.

Greets,

Chi

I have understood your problem already in your 1st mail. My concern
is that the original code is not very good formated! Don't you agree
that my patch is better formated? And thus better readable?

I don't see what formatting you have changed except that you replaced the beautiful tab indentions with simple spaces. You don't leave a blank at the end of a funtion and your empty lines have spaces which are not needed.

Try displaying tabs and spaces in your editor and you might see the difference.

And if you use patch it goes straightly into the is code!!!

Of course it does, but usually I don't want to patch the source only for looking at what you are changing. And obvioulsy your changes affected only 6 lines in a patch of 140 lines. See my point?

And I reuse this patch (or code piece) in my extensions
(ch_is_templates), where I absolutely don't accept code formated like
the original one.

If you don't like the style, you can request to change our coding guidelines. But as long as they are valid, a TYPO3 coder should stick to them:
http://typo3.org/documentation/document-library/doc_core_cgl/

BTW what kind of flags do you use to diff? Your diff has the changes
inline and my diff listen them separately... It would be kind if you told
me this so I can improve my technique.

Hmm, didn't understand that. Technically I think both diffs were created the same way:
diff -ru indexed_search.orig/ indexed_search/ > my_patch.diff

Anyway, I think this is too much OT for staying in this bugtracker. Let's continue discussion off-the-list.

- michael

Hello Michael,

well, I have found out that my prefered editor (scite) is converting tabs to whitespace and so on by default. Because I was wondering why you complaining about whitespaces and so on. Now I know, but I don't understand why the comments are almost everywhere indented one tab more then the code? Looks not very good to me. Or is it a T3 coding guideline?

Greets,

Chi

I couldn't find this in the coding guideline but since it is a generally used practice (look at the rest of the source) I suggest to keep it that way.

Generally: If you are not happy with the style of the code, count up some arguments and start a discussion on the dev list.

I just uploaded a new version of the patch. The old one was wrong as I mixed up two different if clauses (thanks to the nice line indenting, btw! ;-))

The current version seems to work well, I just didn't check if it still counts correct if something is not visible for the current group (missing access)...

I fixed this in CVS although I'm not sure if it's 100% correct now.

The reason for this is that multi-page documents are filtered from the search, however if we are on page 1 for example, all matches after this page are not processed, so we will not know if anything should be filtered out or not.

This could be fixed by traversing all search results even if only the first page needs to be displayed. However I prefer the faster way over the more precise one.