Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix not ending at correct point after <return> #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

smemsh
Copy link
Contributor

@smemsh smemsh commented May 25, 2024

After <CR> in insert mode, the indent is corrected, but we do not resume insert at the beginning of the corrected line, instead we resume at the end of the line, which is not what we want (for cursor to be positioned exactly where we split the line, but on the new line). See complaints in #1. This patch attempts to fix only the <CR> case; we still might want to fix other cases later.

the <END> is needed to cement the additions, apparently
but we need to issue a '^' and then start insert mode, so we're on the
first char of the new line with insert mode active

Note that 'r' to insert a newline does not go through our extension in
any way, so user has to be "aware" to do it using insert and return
we need to distinguish between the <CR> having been pressed at the end
of The line -- in which case, we'll be positioned correctly already --
and in the middle of the line -- in which case, we have to reposition at
the beginning of text on the line, because the cursor is "behind" the
text that used to be in front of it prior to the <CR>
@smemsh
Copy link
Contributor Author

smemsh commented Jun 11, 2024

added a commit which behaves differently depending on whether the happened at the end of a line (ie it will insert a brand new line of text) or in the middle of a line (there will be some text we bring with us hanging after the new cursor position). the repositioning at first nonblank now only occurs in the latter case.

Otherwise, it can result in strange artifacts in quickfix, and in the
cmdline-window mode.  Any buffer with a buffer-local '&buftype' that's
an empty string, is a regular file being edited, where we would like to
be enabled.  Other buftypes, there's no reason for stabs to be enabled
on CR (mostly these are file types which are ephemeral read-only
buffers, cmdline-window mode is really still line-based and enter
modifications don't make sense).  The only one maybe is scratch buffer,
but <CR> would still be usable there and it's better than maintaining a
list of exceptions since just empty buftype solves 99% of use cases
@smemsh
Copy link
Contributor Author

smemsh commented Jun 14, 2024

Added a commit that only implements the extra <CR> logic if the buffer-local option &buftype value is empty, which means it's a regular buffer, not one of the special 'buftype'

intelfx added a commit to intelfx/vim-smarttabs that referenced this pull request Aug 1, 2024
intelfx added a commit to intelfx/vim-smarttabs that referenced this pull request Aug 1, 2024
intelfx added a commit to intelfx/vim-smarttabs that referenced this pull request Aug 1, 2024
@smemsh
Copy link
Contributor Author

smemsh commented Aug 1, 2024

<comment copied from #3 @intelfx>

wrt. #5, why this line?

let l:gotobegin = "\<ESC>:normal!^\<CR>:startinsert\<CR>"

Why not just "<c-o>^"?

At first glance, that seems like it might work. I tried it just now, and it does seems to work fine. Although, it does the same thing (just more concise). I can update my patch if it matters, but it looks like you're using your own version anyways (with other changes), so I'll probably just leave it if you have no plan to use my patch directly, as I'm indifferent about the verbosity of that operation.

@intelfx
Copy link

intelfx commented Aug 5, 2024

but it looks like you're using your own version anyways (with other changes), so I'll probably just leave it if you have no plan to use my patch directly, as I'm indifferent about the verbosity of that operation.

Not necessarily, the plugin I'm using now interacts badly with other things I have — there might be need for further patching, and given so, I guess I'll switch to the better-maintained version (this one).


EDIT: yes, I'm definitely switching to vim-stabs as it offers more control over mappings 😅

@intelfx
Copy link

intelfx commented Aug 5, 2024

(Yeah, sorry, I should not have posted that question on the issue...)

Replying to you over at #3:

what is the point of the motion here?

The next sentence from my post explains:

If the "end" is not given, only a partial insert occurs and the line is incorrect.

Naive cases work with just removing the , but try this C for example (if it matters, the spaces are 8-char tabs):

    if (strcmp(last_component(progname), "more") == 0)
            less_is_more = 1;

put the cursor after the comma, enter insert mode, and use CR. For me, the cursor position on the next line is incorrect after this, after just removing the StabsCR() from vim-stabs master and making no other changes.

I'm not sure what exactly I meant there by "partial insert," and it's been a while, but I had tested a number of cases at the time and only found correct behavior with the sequence I put in my patch. I've been using this ever since and haven't found any issues with it.

So I've tried this example and I can't reproduce the problem if I remove <END> from the mapping. By any chance, do you have a full source file and/or full Vim configuration that reproduces the problem?

FWIW, my Vim is

$ vim --version
VIM - Vi IMproved 9.1 (2024 Jan 02, compiled Aug 01 2024 20:46:30)
Included patches: 1-648
Compiled by Arch Linux
Huge version without GUI.  Features included (+) or not (-):

intelfx added a commit to intelfx/vim-stabs that referenced this pull request Aug 5, 2024
* commit '7fbb97ff3dec9607525b20824718ab469d3741d9':
  Make sure <CR> does not do anything except on regular buffers
  After <CR> behaves differently whether at end of line already
  Fix after CR we should be at the new inserted position with insert on

[intelfx: cleanup, use less verbose map rhs]
intelfx added a commit to intelfx/vim-stabs that referenced this pull request Aug 5, 2024
After `<CR>` in insert mode, the indent is corrected, but we do not
resume insert at the beginning of the corrected line, instead we resume
at the end of the line, which is not what we want (for cursor to be
positioned exactly where we split the line, but on the new line). See
complaints in Thyrum#1. This patch attempts to fix only the `<CR>` case; we
still might want to fix other cases later.

* commit '7fbb97ff3dec9607525b20824718ab469d3741d9':
  Make sure <CR> does not do anything except on regular buffers
  After <CR> behaves differently whether at end of line already
  Fix after CR we should be at the new inserted position with insert on

[intelfx: cleanup, use less verbose map rhs]
intelfx added a commit to intelfx/vim-stabs that referenced this pull request Aug 5, 2024
It does not _seem_ to do anything in my tests...

Link: Thyrum#5 (comment)
@smemsh
Copy link
Contributor Author

smemsh commented Aug 5, 2024

I'm pretty sure the maintainer of this repo is not going to merge my PRs since they already merged a typo fix written at the same time, but made no comment on the other ones I submitted. 🤷 The toggle/on/off one in particular is essential for me, as there are many cases that come up where it needs to be disabled.

The function I gave a sample of was from the https://github.com/gwsw/less repo main.c (search for "last_component").

I just tried it again with vim-stabs master, removing only the <END> in StabsCR(), and I can't get it to work right with even a blank vimrc, so I'm puzzled why we're mismatched. I do need extra settings to get mine working as expected also:
filetype indent on must be enabled, and cinoptions must contain (0 (obviously, regarding C files specifically). I'm using 9.1.573 which is not much different than your version.

In any case, if my version of the patched function works for you also, then I would question the reason to use a simpler one, since in that case my version covers more variants.

intelfx added a commit to intelfx/dotfiles that referenced this pull request Aug 8, 2024
Submodule .vim/bundle/vim-stabs 66a4520..d54457a:
  > plugin: get rid of superfluous `<END>`
  > Merge pull request Thyrum/vim-stabs#5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants