Code review comment for lp:~yann-papouin/ocb-addons/6.1-bug-1267845-product-and-category-search-improved-usability

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

18: add spaces between operators

21: this seems to be one line too much

33: that's not safe against searching for ' / ' or the like, I'd wrap that into an
if category_names:

33, 36: shouldn't strip(product_category_separator + ' ') be enough? (the space just to be safe if we ever change product_category_separator)

42: if ids and classic_ids are nondistinct sets and there's a limit, you'll get less than limit ids, which is particularly annoying for name_search. I think you should pass limit+len(ids), remove duplicates in a manner that preserves class_ids' order and then shorten it as necessary

53: why is this necessary? And if so, why only if name is set?

63: can't you integrate that in the regex above? If you'll only have '[bla' trigger searching for the default code, but not 'bla]', adding a '?' after the closing square bracket would be enough. (and I can't really think of a use case for 'bla]').

64: if you don't agree with 63, .strip('[] ') should be enough here

review: Needs Fixing

« Back to merge proposal