Re: [Nano-devel] adding a word-completion feature to nano

[Sumedh Pendurkar]

Hi,


On Monday 31 October 2016 01:30 AM, Benno Schulenberg wrote:
> > Changed name of pleted_length to pleted_mbchars as it no longer counts
> > bytes it counts characters.
> Why count characters?  If you don't use backspaces to delete
> a tried completion but simply a memmove that obliterates a
> number of bytes, you only need the length of the try, no?
Changed it. Also now we pop one element off the undo stack.
> +    while (is_word_mbchar(&check_line[i], FALSE)) {
> +       len_mbchar = move_mbright(check_line, i);
> +       len_of_word += len_mbchar - i;
> +       i = len_mbchar;
> +       pleted_mbchars++;
> +    }
>
> 'len_mbchar' is wrongly named -- it's not a length, it's a position.
Renamed it. Its just a temporary variable.
> +    while (to_find_start_pos >= 0) {
> +       if (!is_word_mbchar(&openfile->current->data[to_find_start_pos], FALSE)
> +           || !to_find_start_pos)
>
> Oh come on: 'pos >= 0' and then 'if !pos'?  Just use 'pos > 0'.
>
> (And using '!pos' is freaking ugly -- pos is not a boolean value,
> it is a number, so you would use 'pos == 0' instead.)
Sorry for it. Fixed it.
> +    /* Taking care if we reach the start of line */
> +       if (to_find_start_pos)
>
> Your indentation is off.  Please keep things aligned.
Sorry. I should have gone through the code once again.
> Also, you have trailing whitespace elsewhere.  Do you use
> nano to make your edits?  Because then a trailing tab would
> be a glaring block of green.
>
> +               !is_word_mbchar(&pletion_line->data[move_mbleft(pletion_line->data, 
> i)], FALSE)) &&
>
> This looks... absurd.  You're doing this step to the left for every j,
> for every character in the shard, but it is needed only for j == 0.
>
> I've said it before: your logic is convoluted.  What I would do is:
> for every line, run very fast through it, trying to find a match with
> the first byte (byte!) of shard.  Only when you find it, you go to the
> next step: check that the preceding character is non-word-forming.
> If yes, then see if the rest of the bytes (bytes!) of shard match too.
> If yes, then check that the succeeding character is word-forming.
> If so, then copy the completion and so on.
>
> You have three of those steps in a single loop, it seems -- it freaks
> me out, I can't understand it.  And I hate for loops anyway -- one
> uses them to go from here to there, not when looking for something.
I have changed it. added a if statement before the for loop to match the 
first character and check if prev character is whitespace or punctuation 
and then begin with the loop.
> > Please find attachments.
> Please don't send patches you've already sent.  I have them.
> And don't include patches (0003) that have nothing to do with
> completion -- you should have done that on a different branch.
> Please run a 'git rebase master' in your branch, after doing a
> pull on master.
I had some problems while doing rebase. So, I have attached a 
format-patch to initial commit.


Thanks & Regards,
Sumedh Pendurkar

0001-add-a-new-feature-complete.patch
Description: Text Data