----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112584/#review39584 -----------------------------------------------------------
Looks pretty good, except the disregard for the Calligra coding standards. I'll check for deeper issues, if any when you fixed the trivial problems. sheets/ui/CellEditor.h <http://git.reviewboard.kde.org/r/112584/#comment29159> Qt and Calligra uses naming of getters and setters like this: foo() and setFoo() rather than getFoo() and setFoo() sheets/ui/CellEditor.h <http://git.reviewboard.kde.org/r/112584/#comment29160> Two mistakes here: 1. Don't add private members when the class has a d pointer. That defeats the purpose of it, namely to insure forward binary compatibility 2. 'c' is a bad name because it tells you nothing. sheets/ui/CellEditor.cpp <http://git.reviewboard.kde.org/r/112584/#comment29161> Use camel casing: wordCollection sheets/ui/CellEditor.cpp <http://git.reviewboard.kde.org/r/112584/#comment29162> Some formatting here doesn't follow the calligra standards: - Braces on same line as for/while/do/etc - spaces around operators and after ';' - space between 'for' and '(' sheets/ui/CellEditor.cpp <http://git.reviewboard.kde.org/r/112584/#comment29163> Same problem as above with formatting. Check for the same problem in other places, I won't point it out everywhere. sheets/ui/CellEditor.cpp <http://git.reviewboard.kde.org/r/112584/#comment29164> space after ',' sheets/ui/CellEditor.cpp <http://git.reviewboard.kde.org/r/112584/#comment29165> This is unreadable because 'c' is such an obscure name (see above). Also, every member variable should have the 'm_' prefix. - Inge Wallin On Sept. 8, 2013, 2:51 p.m., Jigar Raisinghani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112584/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2013, 2:51 p.m.) > > > Review request for Calligra and Inge Wallin. > > > Description > ------- > > This feature enables you to save time by just clicking/selecting redundant > entries to fill a cell in the sheet. > > How to use: > 1) Open a file containing some data/ Open a blank file and enter some data > 2) Try entering data in the columns which already contain some data which has > atleast first 3 characters similar to some other entry in the same column > > You can see that a popup appears near your cell which gives you suggestions > based on the text you have entered in the current cell. You can chose any > suggestion by clicking on it or using the arrow keys to navigate through > options and using Tab/Enter to fill in the suggestion in the cell. > > Optimization: > 1) Currently, Autocomplete has been optimized to a large extent by Hash Tables > > Further Optimization possible(but not yet done): > 1) Navigate only through cells which are already populated using nextInRow() > and nextInColumn() to populate the HashTable. > 2) Use thread to populate Hash Table when sheet is loaded. This process if > run in background, can optimize the feature further. > > Further Improvements possible: > 1) Currently, the choices filled in the suggestion list are based on Most > Recently Used 3 entries. Autocomplete can be made smarter by populating the > list with 3 Most Redundant entries but this would increase the complexity and > optimization would again be needed. > > > Diffs > ----- > > sheets/ui/CellEditor.h 92bdad6 > sheets/ui/CellEditor.cpp d539c06 > > Diff: http://git.reviewboard.kde.org/r/112584/diff/ > > > Testing > ------- > > The feature works successfully for fair amount of data. Testing yet to be > done for Huge Files. > > > Thanks, > > Jigar Raisinghani > >
_______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
