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 742442 - gb-source-vim: Add support for vim scrolloff
gb-source-vim: Add support for vim scrolloff
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: editor
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-01-06 10:04 UTC by Carlos Soriano
Modified: 2015-01-11 13:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gb-source-vim: Add support for vim scrolloff (31.01 KB, patch)
2015-01-06 10:04 UTC, Carlos Soriano
needs-work Details | Review
gb-source-vim: Add support for vim scrolloff (39.81 KB, patch)
2015-01-06 14:35 UTC, Carlos Soriano
none Details | Review
gb-source-vim: Add support for vim scrolloff (43.81 KB, patch)
2015-01-07 13:11 UTC, Carlos Soriano
none Details | Review
gb-source-vim: Add support for vim scrolloff (43.65 KB, patch)
2015-01-07 13:11 UTC, Carlos Soriano
committed Details | Review

Description Carlos Soriano 2015-01-06 10:04:46 UTC
Vim scrolloff allow the user to set a margin of a number of lines
while moving through the buffer.
Add support to it and add a setting to let the user adjust the number
of lines.

As a detail, a scrolloff greater than half of the number of lines in the
buffer keeps the current line always in the middle.
Comment 1 Carlos Soriano 2015-01-06 10:04:48 UTC
Created attachment 293905 [details] [review]
gb-source-vim: Add support for vim scrolloff
Comment 2 Carlos Soriano 2015-01-06 10:05:29 UTC
Not sure what to do to keep the scrolloff in insert mode thoug... But I guess is working enough for now.
Comment 3 Christian Hergert 2015-01-06 10:43:55 UTC
Review of attachment 293905 [details] [review]:

Couple spelling things and nits. Fairly detailed on the nits because I want you to help maintain at some point :)

-- Christian

::: src/vim/gb-source-vim.c
@@ +81,3 @@
   GPtrArray               *captured_events;
   GbSourceVimMode          mode;
+  GSettings	              *vim_settings;

Lingering tab here.

@@ +110,3 @@
 typedef enum
 {
+  GB_SOURCE_VIM_ALIGMENT_NONE,

Missing N in ALIGNMENT

@@ +114,3 @@
+  GB_SOURCE_VIM_ALIGMENT_TOP,
+  GB_SOURCE_VIM_ALIGMENT_BOTTOM
+} GbSourceVimAligment;

Same.

@@ +228,3 @@
+gb_source_vim_adjust_scroll (GbSourceVim *vim,
+                             gint         line,
+                             GbSourceVimAligment aligment);

Align parameter names

@@ +1551,3 @@
   insert = gtk_text_buffer_get_insert (buffer);
   gtk_text_view_scroll_mark_onscreen (vim->priv->text_view, insert);
+  scroll= gb_source_vim_adjust_scroll (vim, line + 1, GB_SOURCE_VIM_ALIGMENT_NONE);

Same.

@@ +2660,3 @@
+{
+  GdkRectangle rect;
+  gint line_top, line_bottom, line_current;

Each declaration on it's own line.

@@ +2681,3 @@
+  line_bottom = gtk_text_iter_get_line (&iter_bottom);
+  line_current = gtk_text_iter_get_line (&iter_current);
+  page_lines = line_bottom - line_top;

Short circuit if page_lines is zero to avoid divide by zero.

@@ +2691,3 @@
+      result.yalign = CLAMP (yalign, min_yalign, max_yalign);
+      result.line = line;
+      break;

Newline after break.

@@ +5345,3 @@
     g_ptr_array_new_with_free_func ((GDestroyNotify)gdk_event_free);
+
+  vim->priv->vim_settings= g_settings_new ("org.gnome.builder.editor.vim");

space before =
Comment 4 Christian Hergert 2015-01-06 11:14:24 UTC
(In reply to comment #2)
> Not sure what to do to keep the scrolloff in insert mode thoug... But I guess
> is working enough for now.

Can we hook in the key-press-event when in insert mode?
Comment 5 Carlos Soriano 2015-01-06 14:35:02 UTC
Created attachment 293935 [details] [review]
gb-source-vim: Add support for vim scrolloff

Vim scrolloff allow the user to set a margin of a number of lines
while moving through the buffer.
Add support to it and add a setting to let the user adjust the number
of lines.

One downside of the implementation is that movement commands scroll
horizontally not just the needed movement but also scrolls making the
word position the left as possible. This is noticed when the scroll view
has horizontal scrolling and the user moves backward in word motions in
the words that are normally outside the view when the view is scrolled
maximum to the left.

The other downside is that insert mode doesn't take scroll off into
account. But the view position right after entering modal mode. I guess
this is good enough for now.

As a detail, a scrolloff greater than half of the number of lines in the
buffer keeps the current line always in the middle.
Comment 6 Carlos Soriano 2015-01-06 14:36:02 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > Not sure what to do to keep the scrolloff in insert mode thoug... But I guess
> > is working enough for now.
> 
> Can we hook in the key-press-event when in insert mode?

just saw your comment after attaching the new patch. Let me try that then.
Comment 7 Carlos Soriano 2015-01-07 13:11:14 UTC
Created attachment 294028 [details] [review]
gb-source-vim: Add support for vim scrolloff

Vim scrolloff allow the user to set a margin of a number of lines
while moving through the buffer.
Add support to it and add a setting to let the user adjust the number
of lines.

One downside of the implementation is that movement commands scroll
horizontally not just the needed movement but also scrolls making the
word position the left as possible. This is noticed when the scroll view
has horizontal scrolling and the user moves backward in word motions in
the words that are normally outside the view when the view is scrolled
maximum to the left.

The other downside is that insert mode doesn't take scroll off into
account. But the view position right after entering modal mode. I guess
this is good enough for now.

As a detail, a scrolloff greater than half of the number of lines in the
buffer keeps the current line always in the middle.
Comment 8 Carlos Soriano 2015-01-07 13:11:53 UTC
Created attachment 294029 [details] [review]
gb-source-vim: Add support for vim scrolloff

Vim scrolloff allow the user to set a margin of a number of lines
while moving through the buffer.
Add support to it and add a setting to let the user adjust the number
of lines.

One downside of the implementation is that movement commands scroll
horizontally not just the needed movement but also scrolls making the
word position the left as possible. This is noticed when the scroll view
has horizontal scrolling and the user moves backward in word motions in
the words that are normally outside the view when the view is scrolled
maximum to the left.

As a detail, a scrolloff greater than half of the number of lines in the
buffer keeps the current line always in the middle.
Comment 9 Carlos Soriano 2015-01-07 13:14:03 UTC
(In reply to comment #3)
> Review of attachment 293905 [details] [review]:
> 
> Couple spelling things and nits. Fairly detailed on the nits because I want you
> to help maintain at some point :)
> 
> -- Christian
> 
> ::: src/vim/gb-source-vim.c
> @@ +81,3 @@
>    GPtrArray               *captured_events;
>    GbSourceVimMode          mode;
> +  GSettings                  *vim_settings;
> 
> Lingering tab here.
> 
> @@ +110,3 @@
>  typedef enum
>  {
> +  GB_SOURCE_VIM_ALIGMENT_NONE,
> 
> Missing N in ALIGNMENT
> 
> @@ +114,3 @@
> +  GB_SOURCE_VIM_ALIGMENT_TOP,
> +  GB_SOURCE_VIM_ALIGMENT_BOTTOM
> +} GbSourceVimAligment;
> 
> Same.
> 
> @@ +228,3 @@
> +gb_source_vim_adjust_scroll (GbSourceVim *vim,
> +                             gint         line,
> +                             GbSourceVimAligment aligment);
> 
> Align parameter names
> 
> @@ +1551,3 @@
>    insert = gtk_text_buffer_get_insert (buffer);
>    gtk_text_view_scroll_mark_onscreen (vim->priv->text_view, insert);
> +  scroll= gb_source_vim_adjust_scroll (vim, line + 1,
> GB_SOURCE_VIM_ALIGMENT_NONE);
> 
> Same.
> 
> @@ +2660,3 @@
> +{
> +  GdkRectangle rect;
> +  gint line_top, line_bottom, line_current;
> 
> Each declaration on it's own line.
This file doesn't follow the guidelines then, since this file do it like I did it. Changed to your preference (which I noticed is also what you do in other files). Probably this file needs a reformating.
> 
> @@ +2681,3 @@
> +  line_bottom = gtk_text_iter_get_line (&iter_bottom);
> +  line_current = gtk_text_iter_get_line (&iter_current);
> +  page_lines = line_bottom - line_top;
> 
> Short circuit if page_lines is zero to avoid divide by zero.
> 
> @@ +2691,3 @@
> +      result.yalign = CLAMP (yalign, min_yalign, max_yalign);
> +      result.line = line;
> +      break;
> 
> Newline after break.
Same, this file doesn't do that. Changed the part of my code.
> 
> @@ +5345,3 @@
>      g_ptr_array_new_with_free_func ((GDestroyNotify)gdk_event_free);
> +
> +  vim->priv->vim_settings= g_settings_new ("org.gnome.builder.editor.vim");
> 
> space before =
Comment 10 Carlos Soriano 2015-01-07 13:14:46 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > Not sure what to do to keep the scrolloff in insert mode thoug... But I guess
> > is working enough for now.
> 
> Can we hook in the key-press-event when in insert mode?

Indeed, done, and also improved current code which was failing to follow scrolloff in some cases. I'm pretty happy with the result now.
Comment 11 Christian Hergert 2015-01-11 04:08:14 UTC
Sorry for taking so long to push this!

I also added support for ":set scrolloff=10" or using its alias, "so".

Attachment 294029 [details] pushed as c4f71f7 - gb-source-vim: Add support for vim scrolloff
Comment 12 Carlos Soriano 2015-01-11 13:08:07 UTC
(In reply to comment #11)
> Sorry for taking so long to push this!
> 
> I also added support for ":set scrolloff=10" or using its alias, "so".
> 
> Attachment 294029 [details] pushed as c4f71f7 - gb-source-vim: Add support for vim
> scrolloff

Nice! Thanks Christian!