nano-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Benno Schulenberg
Subject: Re: [Nano-devel] adding a word-completion feature to nano
Date: Tue, 01 Nov 2016 17:03:25 +0100

Hello Sumedh,

On Mon, Oct 31, 2016, at 21:23, Sumedh Pendurkar wrote:
> On Monday 31 October 2016 01:30 AM, Benno Schulenberg wrote:
> > 'len_mbchar' is wrongly named -- it's not a length, it's a position.
> 
> Renamed it. Its just a temporary variable.

Yes, it's just a temp var.  But they need a *good* name too.

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

Ehm... really?

+    /* Find the start of the fragment that the user typed. */
+    to_find_start_pos = move_mbleft(openfile->current->data, 
to_find_start_pos);
+    while (to_find_start_pos > 0) {
+       if (!is_word_mbchar(&openfile->current->data[to_find_start_pos], 
FALSE)) {
+           to_find_start_pos++;
+           break;
+       }
+       to_find_start_pos = move_mbleft(openfile->current->data, 
to_find_start_pos);
+    }

Incomprehensible.  And probably incorrect too, because you do
a ++ instead of move_mbright().  But never mind, I've fixed that.

> > 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 use a blank line between quoted text and your text,
to make things easier to parse.]

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

Okay.  But it is still in a style... that is gruesome to me.
See below.

> I had some problems while doing rebase. So, I have attached a 
> format-patch to initial commit.

You will have to learn to use 'git rebase' and resolve conflicts;
in this case you probably needed 'git rebase --skip'.

But never mind now.  I've updated things to my style, have added
a missing correction of openfile->totsize,  and pushed this as a
branch to the repo.  Check it out with:
    git checkout --track origin/word-completion
Base any patches you send off of that branch.

The current code works fine... as long as word wrap does not
kick in.  To see what I mean, run this:

    MALLOC_CHECK_=2 src/nano --ignore --fill=10 +18 NEWS

and type: aa port
then type five times ^], until "No further matches" appears,
then type twice M-U.
Result: Aborted.

I don't know what exactly happens, but possibly it wouldn't
happen if we used backspaces intead of a charmove().
I haven't tried it, though -- no time for that now.

Benno

-- 
http://www.fastmail.com - The way an email service should be




reply via email to

[Prev in Thread] Current Thread [Next in Thread]