Merging auto complete and click to insert patches?

32 views
Skip to first unread message

Christian Johnson

unread,
Jun 3, 2013, 9:10:26 PM6/3/13
to gargl...@googlegroups.com
About 6 months ago merging a patch, Tab Complete, was discussed. For some reason after I fixed the issues that were mentioned, the patch was not merged and I never got a reply. Since then I have reworked the patch to remove the need to press tab; it will now insert completions as you type. Are there any problems in this patch, or can it be merged?

Also the bugs in the Click to Insert patch have been fixed, and also needs review.

https://github.com/RedHatter/granite
https://raw.github.com/RedHatter/granite/master/auto-complete.patch
https://raw.github.com/RedHatter/granite/master/click-insert.patch

Chris Spiegel

unread,
Jun 28, 2013, 5:24:07 PM6/28/13
to gargl...@googlegroups.com
From a purely end-user perspective, I'd like the autocomplete to be
configurable if it's not going to require a keypress to initiate it. I
think a lot of users will find the auto-insertion to be distracting.
I'd prefer a user-initiated autocomplete, but I'm not sure what the
proper solution is with tab not really being available.


Just a couple of small comments on the code:

There are some areas where tabs have sneaked in. These should be
spaces. There are also lines which should be empty but contain
whitespace (tabs and/or spaces). Blank lines should be completely blank.

When dealing with characters, magic numbers should be avoided as much as
possible. For example, when converting to lowercase,

c = c+32

is not clear. Instead,

c = tolower(c)

gives the intent better (and is more portable). In addition, using 0x20
when you really mean ' ' is less portable and less clear.

The issues with characters are kind of murky because Gargoyle is really
dealing with Unicode/Latin-1, so to be completely portable no character
constants should really be used. However, there's almost no chance that
Gargoyle or the interpreters will run properly on a system that is not
ASCII compatible (I know Bocfel won't), so I think it's worth assuming
that C's character handling stuff is ASCII compatible.

Otherwise, with the caveat that I'm not overly familiar with Gargoyle's
graphics code, the patches look good to me, and worked fine when I
tested them.

Chris

Eric Forgeot

unread,
Jun 30, 2013, 9:11:37 AM6/30/13
to gargl...@googlegroups.com
On 28/06/2013 23:24, Chris Spiegel wrote:
> From a purely end-user perspective, I'd like the autocomplete to be
> configurable if it's not going to require a keypress to initiate it. I
> think a lot of users will find the auto-insertion to be distracting.
> I'd prefer a user-initiated autocomplete, but I'm not sure what the
> proper solution is with tab not really being available.
>
>

The same for me. It should probably be configurable in the garglk.ini file.

I'd also prefer it to be enabled only when I press the "tab" key, like
on most IDE or shell systems, and not automatically displaying words
like on the google search engine (one of the reasons I totally stopped
using google search).


Reply all
Reply to author
Forward
0 new messages