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 781342 - [patch] Use visual column based movements
[patch] Use visual column based movements
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: editor
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-04-15 13:01 UTC by Andreas Brauchli
Modified: 2017-04-19 08:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use visual column based movements (23.61 KB, patch)
2017-04-15 13:01 UTC, Andreas Brauchli
none Details | Review
Use visual column based movements (23.75 KB, patch)
2017-04-18 21:01 UTC, Andreas Brauchli
committed Details | Review

Description Andreas Brauchli 2017-04-15 13:01:23 UTC
Created attachment 349889 [details] [review]
Use visual column based movements

Currently, editor movements are offset-bound. i.e. When moving to the previous or next line (arrow-up/down, or j,k keys in vim mode), the editor will try to move the cursor to the same character offset as it was on the line it's moving away from.

This behavior works fine when moving within blocks using the same indentation but results in odd behavior when the indentation changes or when the file uses a messy/broken indentation (mixing tabs and spaces) as the cursor will take a visual side-jump.

Attached is a patch that switches the editor to column based movements. I.e. instead of using the character offset, the visual column is used where the width of a tab is converted to space-equivalents. This results in smoother line movements.

The patch was mostly tested in vim mode, however I did a quick test in the normal gnome-builder mode. Emacs is untested.

Potentially related but not included in the patch:

* Offsets are still used for IdeSourceLocation types (symbol lookups) as the tab width could differ from file to file.

* This might be a prerequisite for block selections (vim)
Comment 1 Christian Hergert 2017-04-17 02:17:49 UTC
This looks good, but it doesn't pass the tests currently for Vim mode.

In particular, create a new file (ctrl+n) and do:

  itesting.
  escape
  dd

To test, cd into tests/

 make test-vim
 ./test-vim

Or optionally `make check` which will take a bit longer...

One that is fixed we can merge it.
Comment 2 Andreas Brauchli 2017-04-17 15:05:32 UTC
Hi Christian

Thanks for the quick review and for pointing out the tests. My apologies for missing those since they're even mentioned in CONTRIBUTING.md :/

It seems that the patch is breaking the select-to-end part of the next-line movement. I'll fix this but it's a behavior that is not vim-native (1). Of course I also understand that gnome-builder's goal isn't a true and complete implementation of vim bindings.

However, If you intend to change this behavior at some point, I can also change the "dd" binding to a sequence that is likely to also work in the future: e.g. "first-char, last-char, next-offset" works.

(1) I'm not sure about emacs' behavior. Quick testing with an online javascript clone (i.e. not pure emacs) seems to suggest that it doesn't go to EOL on next-line when on the last line but that it does go to the first char on prev-line on the first line (vim also doesn't do the latter).

Cheers
Comment 3 Andreas Brauchli 2017-04-18 21:01:37 UTC
Created attachment 350027 [details] [review]
Use visual column based movements

Now next-line moves to end when called on last line. Rebased on master.
Comment 4 Andreas Brauchli 2017-04-18 21:37:59 UTC
Most tests (including test-vim) segfault because of missing libterminal (gnome-ppa doesn't provide libvte-2.91-dev :/)
Comment 5 Christian Hergert 2017-04-19 00:51:25 UTC
Tests seem to pass, thanks for working on this!
Comment 6 Andreas Brauchli 2017-04-19 08:24:19 UTC
Thanks for reviewing and merging so quickly, that's a real pleasure which encourages more contributions.