Review Request 112584: Autocomplete feature for Calligra Sheets

[Inge Wallin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112584/#review40027
-----------------------------------------------------------

Ship it!


Lots of formatting problems.  You should really be more careful when you write the code.

Other than that I didn't see many problems, just a magic number in one case.  So fix the formatting problems and then you can merge.


sheets/ui/CellEditor.h
<http://git.reviewboard.kde.org/r/112584/#comment29617>

    Should have space after comma. There are more places than this one where there isn't but I'm not going to clutter the review by pointing out each and every one.



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29618>

    Also spaces around operators like '='.  You should look at the code around your changes when you add code.



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29619>

    One variable per line.
    
    Also, 'words' and 'wordlist' are a bit confusing so you should probably add a short comment to describe what they store.



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29620>

    spaces around operators and after comma and semicolon.



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29621>

    Hmm, d->complete sounds like a bool.  Perhaps 'completer' would be a better name?



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29622>

    space after 'if', 'while', 'do', 'for'. 



sheets/ui/CellEditor.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29623>

    "it"?  This is a bit vague



sheets/ui/CellToolBase.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29624>

    I know that this isn't in your change, but the doubles should really be qreals.



sheets/ui/CellToolBase.cpp
<http://git.reviewboard.kde.org/r/112584/#comment29625>

    It could be just the web formatting, but it looks as if the indentation is just 2 steps.  If this is not the case, then just ignore this note.


- Inge Wallin


On Sept. 14, 2013, 6:20 p.m., Jigar Raisinghani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112584/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2013, 6:20 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 
>   sheets/ui/CellToolBase.h 0c404ce 
>   sheets/ui/CellToolBase.cpp 512fdbb 
>   sheets/ui/CellToolBase_p.h 9cdadf2 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130914/023d23c6/attachment.htm>