Do not autocomplete if only whitespace precedes cursor

[krageon]

If we are on the first character of a line and the input buffer is empty, return a tab character when autocompleting.

This should fix #254

[krageon]

Travis will probably fail on old php versions as I run only 7.1 locally, I'll fix that after I have lunch.

[bobthecow]

unfortunately this is a public method, so we won't be able to break backwards compatibility. right now.

but! $info['end'] is supposed to be a thing already, and we're using it below. we could work the $end param from the callback into the $info array to augment and/or replace that one, or we could just use that one?

or we could add the $end param to the end here, but that's just a bit awkward.

or we could rename this method and add a deprecated stub of a method called processCallback that still takes the old arguments. i think i like this third option best :)

[krageon]

It's actually the function passed to readline_completion_function that needs the change, which is also public. Refactoring that in a non-breaking manner gives me a large headache, so I reverted it back to the old (and admittedly broken) way. I use point now, which is more in line with what should happen anyway.

[bobthecow]

don't use empty, use === ''. because php.

>>> empty('0')
=> true

[bobthecow]

do all the possible combinations of readline and libedit and php versions all provide $end for this callback? documentation is sadly lacking here :(

if not, should we fall back to the end in readline_info if it's available?

[krageon]

I looked here: https://github.com/php/php-src/blob/master/ext/readline/readline.c#L490

And I saw that params always contains int start and int end.

For editline / libedit, editline/readline.h is included. This has typedef char **rl_completion_func_t(const char *, int, int); and extern rl_completion_func_t *rl_attempted_completion_function;

in editline's readline.c I see this:

/*
 * complete word at current point
 */
/* ARGSUSED */
int
rl_complete(int ignore __attribute__((__unused__)), int invoking_key)
{
	static ct_buffer_t wbreak_conv, sprefix_conv;
	char *breakchars;

	if (h == NULL || e == NULL)
		rl_initialize();

	if (rl_inhibit_completion) {
		char arr[2];
		arr[0] = (char)invoking_key;
		arr[1] = '\0';
		el_insertstr(e, arr);
		return CC_REFRESH;
	}

	if (rl_completion_word_break_hook != NULL)
		breakchars = (*rl_completion_word_break_hook)();
	else
		breakchars = rl_basic_word_break_characters;

	_rl_update_pos();

	/* Just look at how many global variables modify this operation! */
	return fn_complete(e,
	    (rl_compentry_func_t *)rl_completion_entry_function,
	    rl_attempted_completion_function,
	    ct_decode_string(rl_basic_word_break_characters, &wbreak_conv),
	    ct_decode_string(breakchars, &sprefix_conv),
	    _rl_completion_append_character_function,
	    (size_t)rl_completion_query_items,
	    &rl_completion_type, &rl_attempted_completion_over,
	    &rl_point, &rl_end);


}

Where rl_point and rl_end are updated as follows:

static void
_rl_update_pos(void)
{
	const LineInfo *li = el_line(e);

	rl_point = (int)(li->cursor - li->buffer);
	rl_end = (int)(li->lastchar - li->buffer);
}

Which leads me to believe that it does the same thing (http://www.delorie.com/gnu/docs/readline/rlman_28.html tells me that rl_end contains the number of characters in the line).

Even if that is in question, from the readline.c file in php-src, we can see that rl_end (the value passed to the callback as end) and the end key output by readline_info are the same:

https://github.com/php/php-src/blob/master/ext/readline/readline.c#L257

[krageon]

However, having dived into it this far, I cannot help but wonder how this works - I had interpreted end differently than what the documentation suggests it does, and I saw it work in that way as well. The variable I actually need is rl_point, which is always in readline_info but not passed as a callback argument.

[krageon]

The issue that I was fixing is already fixed, so I'm closing this PR.

[bobthecow]

I think this is still valuable for people who don't have a PHP or terminal or readline version that supports bracketed paste… IIRC PHP with libedit doesn't?

[krageon]

Ah. I'm actually experimenting with some changes that will un-break hoa console, they would probably also benefit from this. Alright then, I'll polish this up (probably Monday morning).

…

On Sep 2, 2017 02:21, "Justin Hileman" ***@***.***> wrote: I think this is still valuable for people who don't have a PHP or terminal or readline version that supports bracketed paste… IIRC PHP with libedit doesn't? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#410 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACjTi5SDFUOlvG4RZsLzxmFfblMoO3UXks5seJ-WgaJpZM4PHLK6> .

[krageon]

There. That should be the tests fixed :)

[bobthecow]

because of the magic space inserted after tab completion, this inserts five spaces for the first tab stop, then nine spaces for each additional tab stop. why don't we hijack this and return three spaces? then it's always four space indents :)

[krageon]

It inserts a tab character, not spaces. I'm definitely not a fan of replacing it with space characters, as those are different (no matter what you think of tabs vs spaces) and substituting one for the other silently can never be what you want.

The tab + space behaviour is sort of vexing, but it's not something I can realistically solve in a way that doesn't break everything in environments that do work properly (for example ones that have the path from php/php-src#2720 applied).

What I can probably do is return array("\t" . chr(8));. chr(8) is the backspace character, which should work.

[krageon]

I could add in a setting for this and apply it to every autocompletion, thinking about it. Until the patch I mentioned works that's a sort of viable workaround to get autocompletion as you'd expect from an IDE.

[krageon]

Wait no, that won't work. The space is appended after the completion - so a backspace that is inserted will actually just remove the tab character entirely.

[bobthecow]

i understand that tabs and spaces are different. but practically, one looks like you'd expect, and the other looks really bad. so ¯\_(ツ)_/¯

[GrahamCampbell]

What's the status here?