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 742855 - source-vim: Improve scrolling and clean up
source-vim: Improve scrolling and clean up
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-13 13:46 UTC by Carlos Soriano
Modified: 2015-01-21 20:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
souce-vim: Improve scrolling and clean up (13.53 KB, patch)
2015-01-13 13:46 UTC, Carlos Soriano
none Details | Review
source-vim: Ensure scroll for line0 and line start (1.12 KB, patch)
2015-01-13 13:46 UTC, Carlos Soriano
none Details | Review
source-vim: Ensure scroll for line0 and line start (1.07 KB, patch)
2015-01-13 13:48 UTC, Carlos Soriano
committed Details | Review
[PATCH] souce-vim: Improve scrolling and clean up (13.71 KB, patch)
2015-01-20 09:47 UTC, Christian Hergert
committed Details | Review
vim: remove unnecessary call to gb_source_vim_ensure_scroll() (935 bytes, patch)
2015-01-21 00:33 UTC, Christian Hergert
committed Details | Review
editor-view: focus next split if available upon vim cycle-next (1.22 KB, patch)
2015-01-21 01:05 UTC, Christian Hergert
committed Details | Review
editor-view: focus previous split if available upon vim cycle-previous (1.30 KB, patch)
2015-01-21 01:06 UTC, Christian Hergert
committed Details | Review
screencast of vim and gnome-builder (678.30 KB, video/webm)
2015-01-21 20:49 UTC, Florian Bäuerle
  Details

Description Carlos Soriano 2015-01-13 13:46:26 UTC
Clean up and improve the way we scroll.
Also, add line 0 and line start commands to the ones that need to make
sure about the scrolling.
Comment 1 Carlos Soriano 2015-01-13 13:46:29 UTC
Created attachment 294428 [details] [review]
souce-vim: Improve scrolling and clean up

We need to make sure the scroll even for horizontal movements,
so make sure we do that when no vertical alignment is needed.
Also, as a clean up, the iter bound is always depending of the
selection and if it is a selection present. So just calculate that
inside ensure_scroll.
Comment 2 Carlos Soriano 2015-01-13 13:46:35 UTC
Created attachment 294429 [details] [review]
source-vim: Ensure scroll for line0 and line start

Those also need to make sure the scroll is correct and the cursor
visible

https://bugzilla.gnome.org/show_bug.cgi?id=742854
Comment 3 Carlos Soriano 2015-01-13 13:47:33 UTC
(In reply to comment #2)
> Created an attachment (id=294429) [details] [review]
> source-vim: Ensure scroll for line0 and line start
> 
> Those also need to make sure the scroll is correct and the cursor
> visible
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=742854

ffs, I'm being a noob with git bz file...
Comment 4 Carlos Soriano 2015-01-13 13:48:41 UTC
Created attachment 294430 [details] [review]
source-vim: Ensure scroll for line0 and line start

Those also need to make sure the scroll is correct and the cursor
visible
Comment 5 Christian Hergert 2015-01-20 03:21:17 UTC
This was committed a while ago, forgot to close.
Comment 6 Carlos Soriano 2015-01-20 08:57:42 UTC
(In reply to comment #5)
> This was committed a while ago, forgot to close.

Hi,

Seems it's not
Comment 7 Christian Hergert 2015-01-20 09:47:01 UTC
Created attachment 294963 [details] [review]
[PATCH] souce-vim: Improve scrolling and clean up

We need to make sure the scroll even for horizontal movements,
so make sure we do that when no vertical alignment is needed.
Also, as a clean up, the iter bound is always depending of the
selection and if it is a selection present. So just calculate that
inside ensure_scroll.
Comment 8 Christian Hergert 2015-01-20 09:48:07 UTC
Comment on attachment 294428 [details] [review]
souce-vim: Improve scrolling and clean up

replaced this with forward ported patch
Comment 9 Christian Hergert 2015-01-20 09:50:32 UTC
Okay, pushed for real this time :)

Attachment 294430 [details] pushed as 400f24f - source-vim: Ensure scroll for line0 and line start
Attachment 294963 [details] pushed as c706848 - [PATCH] souce-vim: Improve scrolling and clean up
Comment 10 Carlos Soriano 2015-01-20 09:57:18 UTC
awesome thanks!
Comment 11 Florian Bäuerle 2015-01-20 21:40:18 UTC
I think one of the patches broke goto line through the command bar.

In that case, the line should be

a) centered in the middle of the screen if it was not visible on the current screen
b) the cursor should be positioned to the line, if the line is visible on the current screen

Same should apply to the '123G'-way of jumping to a line in normal mode, those two use different code paths (they also behave a bit differently for the case of line 0).
Comment 12 Christian Hergert 2015-01-21 00:33:30 UTC
Created attachment 295054 [details] [review]
vim: remove unnecessary call to gb_source_vim_ensure_scroll()

I don't think these are necessary. Strictly speaking, they might if you use
the mouse to scroll and then hit escape while in a selection, but that
doesn't bother me as much as it being off for line jumps.
Comment 13 Christian Hergert 2015-01-21 00:34:19 UTC
Comment on attachment 295054 [details] [review]
vim: remove unnecessary call to gb_source_vim_ensure_scroll()

This should fix jumps like ":4000<Return>"

Attachment 295054 [details] pushed as e9fda2a - vim: remove unnecessary call to gb_source_vim_ensure_scroll()
Comment 14 Christian Hergert 2015-01-21 01:05:57 UTC
Created attachment 295056 [details] [review]
editor-view: focus next split if available upon vim cycle-next
Comment 15 Christian Hergert 2015-01-21 01:06:01 UTC
Created attachment 295057 [details] [review]
editor-view: focus previous split if available upon vim cycle-previous
Comment 16 Christian Hergert 2015-01-21 01:06:59 UTC
As always, there are corner cases. But I think this is good enough now to at least consider usable.
I'd like to find an alternate shortcut for closing when under Vim now that ctrl+w is swallowed.

Attachment 295056 [details] pushed as d222542 - editor-view: focus next split if available upon vim cycle-next
Attachment 295057 [details] pushed as ce0b6a9 - editor-view: focus previous split if available upon vim cycle-previous
Comment 17 Carlos Soriano 2015-01-21 10:14:55 UTC
(In reply to comment #11)
> I think one of the patches broke goto line through the command bar.
> 
> In that case, the line should be
> 
> a) centered in the middle of the screen if it was not visible on the current
> screen
> b) the cursor should be positioned to the line, if the line is visible on the
> current screen
> 
> Same should apply to the '123G'-way of jumping to a line in normal mode, those
> two use different code paths (they also behave a bit differently for the case
> of line 0).

I can't reproduce it here or even think what could be the problem you are seeing. With Christian patch it behaves like it behaved previously for me.
Good if now is fixed! But still curious =)
Comment 18 Florian Bäuerle 2015-01-21 20:49:22 UTC
Created attachment 295131 [details]
screencast of vim and gnome-builder

Yes it behaves like it did before. The point i want to make, is that it does not perfectly match the vim behavior.

You can see that in the shown case, vim does place the cursor in line 111, but does not scroll the document. That is all I was trying to say ;)