Skip to content
This repository has been archived by the owner on Nov 11, 2018. It is now read-only.

Issue #381 clickable autocomplete #383

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Autocompletion.vala
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public class Autocompletion : Object {

scrollable_list_view.set_filter_function(filter_function);
scrollable_list_view.set_sort_function(sort_function);
scrollable_list_view.item_hovered.connect(on_item_hovered);
scrollable_list_view.item_clicked.connect(on_item_clicked);
scrollable_list_view.item_double_clicked.connect(on_item_double_clicked);

on_settings_changed(null);
Settings.get_default().changed.connect(on_settings_changed);
Expand Down Expand Up @@ -212,6 +215,20 @@ public class Autocompletion : Object {
stage.set_background_color(Settings.get_default().foreground_color);
}

private void on_item_hovered(int index) {
select_entry(index);
}

private void on_item_clicked(int index) {
select_command(scrollable_list_view.get_item(index).text);
}

private void on_item_double_clicked(int index) {
run_command(scrollable_list_view.get_item(index).text);
}

public signal void run_command(string command);
public signal void select_command(string command);

private class AutocompletionEntry : Object {

Expand Down
10 changes: 10 additions & 0 deletions src/FinalTerm.vala
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ public class FinalTerm : Gtk.Application {
im_context.preedit_end.connect(on_preedit_end);
main_window.key_press_event.connect(on_key_press_event);
main_window.key_release_event.connect(on_key_release_event);
autocompletion.run_command.connect(on_autocomplete_run_command);
autocompletion.select_command.connect(on_autocomplete_select_command);
}

protected override void activate() {
Expand Down Expand Up @@ -243,6 +245,14 @@ public class FinalTerm : Gtk.Application {
return true;
}

private void on_autocomplete_run_command(string command) {
active_terminal_widget.run_shell_command(command);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure what the expected behavior really is here. Maybe on a simple click, the system should actually just put the command in the prompt without executing it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that left click should do the same as pressing enter.
And when someone is using mouse his right hand is on the mouse and not on the keyboard, so to execute a command after click he'll have to move it to the enter button.

But maybe we can put the command in prompt on right click.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think right click should either provide a menu or do nothing at all. Single click should copy the command in the prompt, double click should run the command. I quite sure about the double click though. Mouse usage is not supposed to be fast anyway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any reference to double click event in Clutter. Is there one?
If there is not, it should be implemented for the whole of finalterm not just autocomplete, because it will be needed in other cases (text selection by words, for example).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any reference to double click event in Clutter. Is there one?

That's a very good question. I spent quite a while looking, and indeed I found that the Clutter.ButtonEvent object that you receive in the signal has a click_count property that counts the number of clicks in that "click sequence", so checking whether this is >= 2 should do the trick.

However, the docs have something weird to say about this: "The clicks do not have to occur on the same actor: providing they occur within the double click distance and time, they are counted as part of the same click sequence."

Still, this is obviously intended to provide double-click support so we should probably just use it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also agree with @arkocal's proposal: Single click should copy the command into the prompt, double click should run it, and right/middle/etc. clicks should do nothing.

}

private void on_autocomplete_select_command(string command) {
active_terminal_widget.set_shell_command(command);
}

private bool on_key_release_event(Gdk.EventKey event) {
return im_context.filter_keypress(event);
}
Expand Down
56 changes: 56 additions & 0 deletions src/ScrollableListView.vala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public class ScrollableListView<T, E> : Clutter.Actor {

private Clutter.Model list_model;

private bool process_click = false;

public ScrollableListView(NotifyingList<T> list, Type item_type, Type item_view_type, string item_property_name) {
scroll_view = new Mx.ScrollView();
add(scroll_view);
Expand Down Expand Up @@ -54,6 +56,8 @@ public class ScrollableListView<T, E> : Clutter.Actor {
list_view.add_attribute(item_property_name, 0);

scroll_view.add(list_view);
scroll_view.motion_event.connect(on_motion_event);
scroll_view.button_press_event.connect(on_button_press_event);;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doubled semicolons


// Synchronize model with list
foreach (var item in list) {
Expand Down Expand Up @@ -143,11 +147,63 @@ public class ScrollableListView<T, E> : Clutter.Actor {
scroll_view.ensure_visible(geometry);
}

public int get_item_by_y(int y) {
var index = -1;
var height = 0;
for (int i = 0; i < get_number_of_items(); i++) {
var allocation_box = get_item_view(i).get_allocation_box();
height += (int)allocation_box.get_height();
if ((int)y < height) {
index = i;
break;
}
}
return index;
}

private void on_settings_changed(string? key) {
scroll_view.style = Settings.get_default().theme.style;
list_view.style = Settings.get_default().theme.style;
}

private bool on_motion_event(Clutter.MotionEvent event) {
var index = get_item_by_y((int)event.y);
if (index >= 0) {
item_hovered(index);
}

return true;
}

private bool on_button_press_event(Clutter.ButtonEvent event) {
int index = get_item_by_y((int)event.y);
process_click = false;

if (index >= 0) {
if (event.click_count == 1) {
process_click = true;
ScrollableListView instance = this;
// Button press event fires twice on double click
// because of this the processing of the first click should be delayed
// if the second click has happened in the meantime process_click would be false
// TODO: look for a better solution
Timeout.add(Clutter.Settings.get_default().double_click_time + 50, () => {
if (instance.process_click) {
instance.item_clicked(index);
}
return false;
}, Priority.DEFAULT);
} else if (event.click_count > 1) {
item_double_clicked(index);
}
}

return true;
}

public signal void item_hovered(int index);
public signal void item_clicked(int index);
public signal void item_double_clicked(int index);

private class ItemViewFactory<G> : Object, Mx.ItemFactory {

Expand Down
2 changes: 1 addition & 1 deletion src/Terminal.vala
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public class Terminal : Object {
}

private void on_output_command_updated(string command) {
message(_("Command updated: '%s'"), command);
// message(_("Command updated: '%s'"), command);

var stripped_command = command.strip();
if (stripped_command == "") {
Expand Down