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

[Benno Schulenberg]

On Sun, Oct 30, 2016, at 18:24, Sumedh Pendurkar wrote:
> Now it should work fine.

After a quick test, it seems it does.

> 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?

+    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.

+    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.)

+    /* Taking care if we reach the start of line */
+       if (to_find_start_pos)

Your indentation is off.  Please keep things aligned.

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.

> 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.

Benno

-- 
http://www.fastmail.com - Access all of your messages and folders
                          wherever you are