After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 748652 - vim command-bar: various fixes and enhancements
vim command-bar: various fixes and enhancements
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-29 15:30 UTC by sébastien lafargue
Modified: 2015-04-29 20:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
command-bar: more columns in the command list (2.92 KB, patch)
2015-04-29 15:31 UTC, sébastien lafargue
committed Details | Review
vim: fix jump to line in command bar (4.98 KB, patch)
2015-04-29 15:32 UTC, sébastien lafargue
committed Details | Review
command bar: better discovering of commands (1.60 KB, patch)
2015-04-29 15:32 UTC, sébastien lafargue
committed Details | Review
vim command bar: allow to override GAction commands (9.42 KB, patch)
2015-04-29 15:33 UTC, sébastien lafargue
committed Details | Review
gaction-provider: fix discovering of groups (1.33 KB, patch)
2015-04-29 15:33 UTC, sébastien lafargue
committed Details | Review
gaction-provider: filtering of duplicates (1.19 KB, patch)
2015-04-29 15:33 UTC, sébastien lafargue
committed Details | Review
command-gaction-provider: add debug helper (3.94 KB, patch)
2015-04-29 15:33 UTC, sébastien lafargue
committed Details | Review
example of actions debug output (2.47 KB, text/plain)
2015-04-29 15:39 UTC, sébastien lafargue
  Details
command-bar: fix somes errors (1.13 KB, patch)
2015-04-29 20:53 UTC, sébastien lafargue
committed Details | Review

Description sébastien lafargue 2015-04-29 15:30:24 UTC
see the description in attachements
Comment 1 sébastien lafargue 2015-04-29 15:31:48 UTC
Created attachment 302576 [details] [review]
command-bar: more columns in the command list

Use of a higher number of max columns in command list
(shown with tab in command bar)
Comment 2 sébastien lafargue 2015-04-29 15:32:35 UTC
Created attachment 302577 [details] [review]
vim: fix jump to line in command bar

:line_number to jump to a line number now work
Comment 3 sébastien lafargue 2015-04-29 15:32:49 UTC
Created attachment 302578 [details] [review]
command bar: better discovering of commands

Actualy, we use two providers for discover the commands
but the GAction provider gives us too much commands
with duplicates, so we need to filter it better.
Comment 4 sébastien lafargue 2015-04-29 15:33:00 UTC
Created attachment 302579 [details] [review]
vim command bar: allow to override GAction commands

As GAction discovering give us duplicate names for commands,
we need a way to circumvent that problem:

In gb-command-gaction-provider, GbActionCommandMap action_maps []
gives us a way to define custom commands and the prefix/action name
they mask.
Comment 5 sébastien lafargue 2015-04-29 15:33:12 UTC
Created attachment 302580 [details] [review]
gaction-provider: fix discovering of groups

We remove the workench manual entry as it's already
discover by the widgets hierarchy traversal.
Comment 6 sébastien lafargue 2015-04-29 15:33:26 UTC
Created attachment 302581 [details] [review]
gaction-provider: filtering of duplicates

Some commands are duplicates, cmming from different widgets
but with the same name :

The global quit is renamed to quitall
close, save and save-as of view is already handle by view-stack,
so we masks them.
Comment 7 sébastien lafargue 2015-04-29 15:33:38 UTC
Created attachment 302582 [details] [review]
command-gaction-provider: add debug helper

To better trace the existing GAction for the entire
application, we can use this helper : show_prefix_actions ()

Change #if 0 by #if 1 to activate it

( you can find them at the top of gb-command-gaction-provider.c file :

	#if 0
	#define GB_DEBUG_ACTIONS

To get the result in your terminal, show the command bar with ':'
then hit 'tab' ( like you do to list all the possible commands )

The result can be nicely seen as a .yaml file with syntax highlight
in all GtkSourceVIew based application.
Comment 8 sébastien lafargue 2015-04-29 15:39:54 UTC
Created attachment 302583 [details]
example of actions debug output
Comment 9 Christian Hergert 2015-04-29 19:28:16 UTC
Review of attachment 302576 [details] [review]:

LGTM
Comment 10 Christian Hergert 2015-04-29 19:29:18 UTC
Review of attachment 302577 [details] [review]:

LGTM
Comment 11 Christian Hergert 2015-04-29 19:31:37 UTC
Review of attachment 302578 [details] [review]:

Fix the GType issue and then good to go.

::: src/commands/gb-command-gaction-provider.c
@@ -44,2 +47,3 @@
   GtkWidget *widget;
   GList *list = NULL;
+  gint type;

I just learned recently that this does in fact need to be GType, or else weird things can happen with -fPIC/-fPIE. (See Gom bug about crashes).

@@ +61,3 @@
 
+      /* We exclude these types, they're already in the widgets hierarchy */
+      type = G_OBJECT_TYPE (widget);

Generally I use G_TYPE_FROM_INSTANCE(), but looks like this is just an alias for that.
Comment 12 Christian Hergert 2015-04-29 19:37:53 UTC
Review of attachment 302579 [details] [review]:

LGTM

::: src/commands/gb-command-gaction-provider.c
@@ +204,3 @@
+ */
+static const GbActionCommandMap action_maps [] = {
+  { NULL, NULL, NULL }

{ NULL } suffixes, C (since pre-C89) expands all items to zero that are not specified.

@@ +289,1 @@
+          if (g_str_equal (prefix, gb_group->prefix) && g_action_group_has_action (group, action_name))

Can prefix ever be NULL here? Should we use (g_strcmp0()==0) instead?

Optionally, I'd be happy to have ide_str_equal0() that is a g_str_equal() that allows for NULL.
Comment 13 Christian Hergert 2015-04-29 19:38:36 UTC
Review of attachment 302580 [details] [review]:

LGTM
Comment 14 Christian Hergert 2015-04-29 19:39:29 UTC
Review of attachment 302581 [details] [review]:

LGTM
Comment 15 Christian Hergert 2015-04-29 19:41:32 UTC
Review of attachment 302582 [details] [review]:

LGTM. I wonder if long term we should make tracing able to be broken up into subsystems. (or at least which ones will log to stdout/file). Then we could just have this inside of something like:

#ifdef IDE_ENABLE_TRACE
...
#endif
Comment 16 sébastien lafargue 2015-04-29 20:53:18 UTC
Created attachment 302611 [details] [review]
command-bar: fix somes errors

According to the reviews for preceding patches.
Comment 17 sébastien lafargue 2015-04-29 20:55:24 UTC
Attachment 302576 [details] pushed as 62b90a4 - command-bar: more columns in the command list
Attachment 302577 [details] pushed as a901b1e - vim: fix jump to line in command bar
Attachment 302578 [details] pushed as 8058e98 - command bar: better discovering of commands
Attachment 302579 [details] pushed as 18c7f17 - vim command bar: allow to override GAction commands
Attachment 302580 [details] pushed as 02987c7 - gaction-provider: fix discovering of groups
Attachment 302581 [details] pushed as f5bc16e - gaction-provider: filtering of duplicates
Attachment 302582 [details] pushed as 4748af9 - command-gaction-provider: add debug helper
Attachment 302611 [details] pushed as 4194689 - command-bar: fix somes errors