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

Issue #381 clickable autocomplete #383

Open
Eugene-msc 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 3 commits
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
13 changes: 12 additions & 1 deletion 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 @@ -122,7 +124,7 @@ public class Autocompletion : Object {
return scrollable_list_view.is_valid_item_index(selected_index);
}

public string? get_selected_command() {
public string? scrollable_list_view.get_item(selected_index).text {

@p-e-w p-e-w Nov 15, 2014

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is the weirdest construct I have ever seen in Vala. I am completely unfamiliar with such syntax. How does this compile?!? Can you enlighten me?

It's not a method (no parentheses, unless the ones around selected_text are somehow counted), it's not a property (no get/set blocks), so what is it?

if (is_command_selected())
return scrollable_list_view.get_item(selected_index).text;

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(scrollable_list_view.get_item(index).text);
}

public signal void run_command(string command);

private class AutocompletionEntry : Object {

Expand Down
5 changes: 5 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,7 @@ 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);
}

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

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

@p-e-w p-e-w Nov 9, 2014

Copy link
Copy Markdown
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.

@Eugene-msc Eugene-msc Nov 9, 2014

Copy link
Copy Markdown
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.

@arkocal arkocal Nov 9, 2014

Copy link
Copy Markdown
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.

@Eugene-msc Eugene-msc Nov 11, 2014

Copy link
Copy Markdown
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).

@p-e-w p-e-w Nov 15, 2014

Copy link
Copy Markdown
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.

@p-e-w p-e-w Nov 15, 2014

Copy link
Copy Markdown
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
36 changes: 36 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);;

@p-e-w p-e-w Nov 9, 2014

Copy link
Copy Markdown
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,45 @@ 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();

@p-e-w p-e-w Nov 9, 2014

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Spacing issues

@p-e-w p-e-w Nov 15, 2014

Copy link
Copy Markdown
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) {
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