Merge lp:~jeremy-munsch/synapse-project/native-de-shortcut into lp:synapse-project

Proposed by Jérémy Munsch
Status: Merged
Merged at revision: 631
Proposed branch: lp:~jeremy-munsch/synapse-project/native-de-shortcut
Merge into: lp:synapse-project
Diff against target: 107 lines (+47/-0)
3 files modified
src/core/utils.vala (+26/-0)
src/ui/controller.vala (+17/-0)
src/ui/keybindings.vala (+4/-0)
To merge this branch: bzr merge lp:~jeremy-munsch/synapse-project/native-de-shortcut
Reviewer Review Type Date Requested Status
Rico Tzschichholz Approve
Review via email: mp+277817@code.launchpad.net

Description of the change

Add shortcut ctrl+backspace for delete word

To post a comment you must log in.
Revision history for this message
Rico Tzschichholz (ricotz) wrote :

https://paste.debian.net/plain/333517

"remove_last_word" is not working correctly so please test this part more.

"Hello you" > ""
"Hello you " > "Hello"
-> both should result in "Hello " or "Hello"?

review: Needs Fixing
Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :

Well i just tested:

"Hello you" > "Hello" *OK*
"Hello you " > "Hello you" *NO OK*

also :
"Hello" > "" *OK*
"Hello " > "" *NO OK*
"Hello you little" > "Hello you" *OK*
"Hello you little " > "Hello you little" *NO OK*

I cannot reproduce "Hello you" > "", this is unwanted behaviour indeed.

I'll make corrections

Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :

Sorry i mistaped test:

"Hello " > "Hello" *NO OK*

Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :

launchpad is removing multiple spaces, but it check words like '_' == ' '
"Hello_you______little" > "Hello_you" *OK*
"Hello_you________little_" > "Hello_you ________little" *NO OK*

Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Ok, looks like it gets called twice on my system, so "remove_last_word" is called to often but what it does is correct.

Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :
Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :

> Ok, looks like it gets called twice on my system, so "remove_last_word" is
> called to often but what it does is correct.

That may be the input method again, this is causing lot of troubles (é_è) <-- sad dude

Which input metod are you using ? Does using GTK_IM_METHOD changes this ?
I am using GTK_IM_METHOD='' synapse on my system, otherwise it crash, this is also a workaround for #1500912

Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Hmm, right, here it is the default: GTK_IM_MODULE="ibus"
With GTK_IM_MODULE="" it works as it is intended.

Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Do not make the argument owned.

The trailing space should be preserved after cutting off the last word.

review: Needs Fixing
Revision history for this message
Rico Tzschichholz (ricotz) wrote :

I have taken a look at the remove_last_word method.

It should work in all cases that way
https://paste.debian.net/plain/333745

Jfyi, regarding "space_index = -1 == space_index ? 0 : space_index+1;". You can't simply do "space_index+1" since this might not be a valid byte-position in this utf8 string.

review: Needs Fixing
631. By Jérémy Munsch

Add shortcut ctrl+backspace for delete word

Revision history for this message
Jérémy Munsch (jeremy-munsch) wrote :

Ok thank you, I applied the patch.

> Jfyi, regarding "space_index = -1 == space_index ? 0 : space_index+1;". You
> can't simply do "space_index+1" since this might not be a valid byte-position
> in this utf8 string.

Indeed I haven't taught about that.

Revision history for this message
Rico Tzschichholz (ricotz) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/core/utils.vala'
--- src/core/utils.vala 2015-11-15 17:19:04 +0000
+++ src/core/utils.vala 2015-11-19 09:11:53 +0000
@@ -54,6 +54,32 @@
54 return input.substring (0, len);54 return input.substring (0, len);
55 }55 }
5656
57 public static string remove_last_word (string input)
58 {
59 long char_count = input.char_count ();
60 int index = input.index_of_nth_char (char_count - 1);
61 unichar pchar = input.get_char (index);
62
63 while (pchar == ' ' && index > 0)
64 {
65 input.get_prev_char (ref index, out pchar);
66 }
67
68 int next_index = index;
69
70 do
71 {
72 index = next_index;
73 input.get_prev_char (ref next_index, out pchar);
74 }
75 while (pchar != ' ' && next_index > 0);
76
77 if (next_index <= 0)
78 return "";
79 else
80 return input.slice (0, index);
81 }
82
57 public static async bool query_exists_async (GLib.File f)83 public static async bool query_exists_async (GLib.File f)
58 {84 {
59 bool exists;85 bool exists;
6086
=== modified file 'src/ui/controller.vala'
--- src/ui/controller.vala 2014-07-10 11:39:28 +0000
+++ src/ui/controller.vala 2015-11-19 09:11:53 +0000
@@ -231,6 +231,14 @@
231 }231 }
232 }232 }
233233
234 protected void search_delete_word ()
235 {
236 if (model.query[model.searching_for] == "") return;
237 model.query[model.searching_for] = Synapse.Utils.remove_last_word (model.query[model.searching_for]);
238
239 search ();
240 }
241
234 protected void search_add_delete_char (string? newchar = null)242 protected void search_add_delete_char (string? newchar = null)
235 {243 {
236 if (newchar == null)244 if (newchar == null)
@@ -246,6 +254,12 @@
246 model.query[model.searching_for] =254 model.query[model.searching_for] =
247 model.query[model.searching_for] + newchar;255 model.query[model.searching_for] + newchar;
248 }256 }
257
258 search ();
259 }
260
261 void search ()
262 {
249 switch (model.searching_for)263 switch (model.searching_for)
250 {264 {
251 case SearchingFor.SOURCES:265 case SearchingFor.SOURCES:
@@ -299,6 +313,9 @@
299 case KeyComboConfig.Commands.SEARCH_DELETE_CHAR:313 case KeyComboConfig.Commands.SEARCH_DELETE_CHAR:
300 search_add_delete_char ();314 search_add_delete_char ();
301 break;315 break;
316 case KeyComboConfig.Commands.SEARCH_DELETE_WORD:
317 search_delete_word ();
318 break;
302 case KeyComboConfig.Commands.CLEAR_SEARCH_OR_HIDE:319 case KeyComboConfig.Commands.CLEAR_SEARCH_OR_HIDE:
303 clear_search_or_hide_pressed ();320 clear_search_or_hide_pressed ();
304 break;321 break;
305322
=== modified file 'src/ui/keybindings.vala'
--- src/ui/keybindings.vala 2014-07-10 11:39:28 +0000
+++ src/ui/keybindings.vala 2015-11-19 09:11:53 +0000
@@ -31,6 +31,7 @@
31 ACTIVATE,31 ACTIVATE,
32 INVALID_COMMAND,32 INVALID_COMMAND,
33 SEARCH_DELETE_CHAR,33 SEARCH_DELETE_CHAR,
34 SEARCH_DELETE_WORD,
34 NEXT_RESULT,35 NEXT_RESULT,
35 PREV_RESULT,36 PREV_RESULT,
36 NEXT_CATEGORY,37 NEXT_CATEGORY,
@@ -55,6 +56,7 @@
55 public string execute { get; set; default = "Return"; }56 public string execute { get; set; default = "Return"; }
56 public string execute_without_hide { get; set; default = "<Shift>Return"; }57 public string execute_without_hide { get; set; default = "<Shift>Return"; }
57 public string delete_char { get; set; default = "BackSpace"; }58 public string delete_char { get; set; default = "BackSpace"; }
59 public string delete_word { get; set; default = "<Control>BackSpace"; }
58 public string alternative_delete_char { get; set; default = "Delete"; }60 public string alternative_delete_char { get; set; default = "Delete"; }
59 public string next_match { get; set; default = "Down"; }61 public string next_match { get; set; default = "Down"; }
60 public string prev_match { get; set; default = "Up"; }62 public string prev_match { get; set; default = "Up"; }
@@ -168,6 +170,8 @@
168 kcs.set_keycombo_command (keyval, mods, Commands.SEARCH_DELETE_CHAR);170 kcs.set_keycombo_command (keyval, mods, Commands.SEARCH_DELETE_CHAR);
169 name_to_key_mod (alternative_delete_char, out keyval, out mods);171 name_to_key_mod (alternative_delete_char, out keyval, out mods);
170 kcs.set_keycombo_command (keyval, mods, Commands.SEARCH_DELETE_CHAR);172 kcs.set_keycombo_command (keyval, mods, Commands.SEARCH_DELETE_CHAR);
173 name_to_key_mod (delete_word, out keyval, out mods);
174 kcs.set_keycombo_command (keyval, mods, Commands.SEARCH_DELETE_WORD);
171 name_to_key_mod (next_match, out keyval, out mods);175 name_to_key_mod (next_match, out keyval, out mods);
172 kcs.set_keycombo_command (keyval, mods, Commands.NEXT_RESULT);176 kcs.set_keycombo_command (keyval, mods, Commands.NEXT_RESULT);
173 name_to_key_mod (prev_match, out keyval, out mods);177 name_to_key_mod (prev_match, out keyval, out mods);