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

Issue #381 clickable autocomplete #383

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dec82ff
autocomplete window now works with mouse
Eugene-msc Nov 8, 2014
6048afa
codestylefixes
Eugene-msc Nov 9, 2014
db715af
autocomplete runs command on click using provided index rather than t…
Eugene-msc Nov 9, 2014
b1cae98
fix. sorry
Eugene-msc Nov 15, 2014
5e4f7cb
implementation on click (print command), doubleclick (execute command…
Eugene-msc Nov 15, 2014
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
11 changes: 11 additions & 0 deletions src/Autocompletion.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ 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);

on_settings_changed(null);
Settings.get_default().changed.connect(on_settings_changed);
Expand Down Expand Up @@ -212,6 +214,15 @@ 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) {
run_command(get_selected_command());
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than running get_selected_command and relying on the hovering to select the item before we can click on it, this should retrieve the command to run based on index.

Actually, I don't think we should have hover-highlighting at all (most autocompletion systems don't).

}

public signal void run_command(string command);

private class AutocompletionEntry : Object {

Expand Down
8 changes: 8 additions & 0 deletions src/FinalTerm.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ 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);

Copy link
Owner

Choose a reason for hiding this comment

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

Unnecessary blank lines (there should be only one on top and none on bottom).

}

protected override void activate() {
Expand Down Expand Up @@ -243,6 +247,10 @@ 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 bool on_key_release_event(Gdk.EventKey event) {
return im_context.filter_keypress(event);
}
Expand Down
34 changes: 34 additions & 0 deletions src/ScrollableListView.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,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 +145,43 @@ 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 (var i = 0;i < get_number_of_items();i++) {
Copy link
Owner

Choose a reason for hiding this comment

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

Spacing issues

var allocation_box = get_item_view(i).get_allocation_box();
height += (int)allocation_box.get_height();
Copy link
Owner

Choose a reason for hiding this comment

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

Spacing issues

Copy link
Owner

Choose a reason for hiding this comment

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

There is still an extra space after the operator here

if((int)y < height) {
Copy link
Owner

Choose a reason for hiding this comment

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

Here also. In general, please use a space after if.

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) {
var index = get_item_by_y((int)event.y);
if(index >= 0)
item_clicked(index);

return true;
}

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

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

Expand Down