DISCLAIMER: This is going to be a loooong reading... Hi all, I was looking for some cleanup in kkstrtext and I find it difficult to understand the purpose of a couple functions, namely rtabmargin and ltabmargin: you can find them in kkstrtext/kkstrtext.cc, line 557. By the name of these functions it's not possible to understand what is their intended purpose and by looking at the implementation it seems wrong; here is why I think so: The function signature is:
int rtabmargin(bool fake, int curpos, const char *p = 0); int ltabmargin(bool fake, int curpos, const char *p = 0); the character pointer is a text string, curpos is the position of a cursor within that string (no checks on that), fake... will be explained later (basically: if it's true, a tab is made of 4 spaces, otherwise 8). First of all I'd like to share with you my tests and results. Here is the source code [1] of the test and the results [2] are here. [1] http://cim.sbisinger.net/kkstrtest.cc [2] http://cim.sbisinger.net/results.txt Observe that the results for ltabmargin with the third parameter do not differ whether fake is true or false. This is because we have this in the function: if(fake) { near = (curpos/(TAB_SIZE/2))*(TAB_SIZE/2); if(near <= curpos-n) if((ret = curpos-n) != 0) ret++; } near = (curpos/TAB_SIZE)*TAB_SIZE; if(near <= curpos-n) { if((ret = curpos-n) != 0) ret++; } else ret = near; Let me simplify this for you: if(fake) { near = curpos % (TAB_SIZE/2); if(near >= n) if((ret = curpos-n) != 0) ret++; } near = curpos % TAB_SIZE; if(near >= n) { if((ret = curpos-n) != 0) ret++; } else ret = near; First of all, being near1 the first near and near2 the second one, near1 <= near2, so if the first if(near >= n) is true, so it is the second one, thus overwriting any value for ret. And anyway, even if it wasn't, ret is being overwritten anyway. So the only possible values for ret are: curpos-n+1, 0, near. This is surely not the intended behaviour for this function. The results for rtabmargin do differ slightly between fake = true and fake = false, in the latter case is skips positions which are not % 8 == 0 (confront the results to understand what I am talking about), but I can notice some inconsistencies: first of all, why spaces which are in a position % TAB_SIZE (or %(TAB_SIZE/2)) do return something different than -1? Why spaces at the end of the string return some other values? For instance: space at position 55 (starting from 0) returns 56 (the number of characters up to that position), but space at position 67 returns 72 (wtf?). Without the third argument the functions return respectively the left and right margin of a tab, given the position in the string. I think that this behaviour is correct. What I think the functions were meant to be is this: l(r)tabmargin should return the left(right) limit of the current tab if before the given position there are only spaces and between ltabmargin and rtabmargin there are only spaces, -1 otherwise. I am not completely sure that this was the intended behaviour and I am even less conviced by the usage of these functions inside the actual code (in kkconsui/abstractconsui.cc). Can anyone else have a look at this and let me know if I am completely wrong or if I make some wrong assumptions? Thank you -- Stéphane -- _______________________________________________ Centerim-devel mailing list Centerim-devel@centerim.org http://centerim.org/mailman/listinfo/centerim-devel http://www.centerim.org/
