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

defaults: set nostartofline #11135

Merged
merged 1 commit into from Dec 4, 2019
Merged

defaults: set nostartofline #11135

merged 1 commit into from Dec 4, 2019

Conversation

teto
Copy link
Member

@teto teto commented Oct 1, 2019

Having the cursor change column is surprising.

I haven't found anything that disturbed me running nostartofline in my init.vim.
@justinmk could you precise the annoying scenarios you are confronted with in tpope/vim-sensible#60 ?

@justinmk
Copy link
Member

justinmk commented Oct 2, 2019

  • Need to update test/functional/helpers.lua (and probably some individual test setup routines)
  • Needs mention in vim_diff.txt

@justinmk could you precise the annoying scenarios you are confronted with in tpope/vim-sensible#60 ?

@teto my intention there was only to avoid a "drastic" change. I've since been using nostartofline for years without issue, and it is indeed superior to the Vi behavior.

@justinmk justinmk added the defaults issues or PRs involving changing the defaults label Oct 2, 2019
@justinmk
Copy link
Member

justinmk commented Nov 25, 2019

  • Needs mention in vim_diff.txt
  • failing test: test/functional/legacy/breakindent_spec.lua

@teto
Copy link
Member Author

teto commented Dec 1, 2019

is that ok if I modernize the test/functional/legacy/breakindent_spec.lua to itemize tests ?

@justinmk
Copy link
Member

justinmk commented Dec 1, 2019

is that ok if I modernize the test/functional/legacy/breakindent_spec.lua to itemize tests ?

the feed_command calls could be one big helpers.source, but otherwise it's best to keep it similar to the original oldtest. I guess we actually don't need it since it might be redundant with src/nvim/testdir/test_breakindent.vim (but then one should check the git history carefully)

@@ -129,7 +129,7 @@ describe('breakindent', function()
feed_command('let g:test="Test 15: breakindent + visual blockwise delete #2"')
feed_command('%d')
feed_command('normal! 4a1234567890')
feed_command([[exec "normal! >>\<C-V>3f0x"]])
feed_command([[exec "normal! >>^\<C-V>3f0x"]])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be necessary. I think what is happening is, the set all& call on line 95 is setting the wrong default. because options.lua should set vim=false

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with nostartofline, the cursor remains at the end of the insertion after normal! 4a1234567890 whereas with startofline, it puts it at the beginning of the line so setting the correct default would also require my modification I think.
if I start the binary, :verbose set startofline? returns nostartofline.
I confess I am kinda confused by the values in options.lua, I assume I should have in the end the following behavior:

:set startofline&vi -> startofline
:set startofline&vim -> startofline
:set startofline& -> nostartofline

?

Copy link
Member

@justinmk justinmk Dec 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:set startofline&vim -> startofline

? That would contradict the actual default. Although it's debatable, up to now we've updated "vim defaults" to whatever our defaults are. Otherwise we would need to introduce &nvim variant for :set.

In any case, it would make more senes to set the option after :set all& rather than update various :normal invocations in the test....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise we would need to introduce &nvim variant for :set.

I've thought about that being useful myself before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blueyed yeah, it's probably a good idea. Then we could use :set all&vim as a way of checking "vim defaults" against user reports.

src/nvim/options.lua Outdated Show resolved Hide resolved
@blueyed
Copy link
Contributor

blueyed commented Dec 2, 2019

I guess we actually don't need it since it might be redundant with src/nvim/testdir/test_breakindent.vim (but then one should check the git history carefully)

👍

@@ -14,6 +14,7 @@ func! Test_edit_00b()
call setline(1, ['abc '])
inoreabbr <buffer> h here some more
call cursor(1, 4)
set startofline
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this make more sense in src/nvim/testdir/setup.vim ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed & done.

@@ -261,7 +262,7 @@ function Test_breakindent15()
call s:test_windows('setl breakindent briopt= ts=8 sw=8')
vert resize 30
norm! 4a1234567890
exe "normal! >>\<C-V>3f0x"
exe "normal! >>^\<C-V>3f0x"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still wondering why we are tweaking random test actions instead of setting the default.

Having the cursor change column can be surprising.

Force startofline in functional and old tests.
Remove the functional breakindent test, as it's a subset of the oldtest one.
@justinmk justinmk mentioned this pull request Dec 4, 2019
20 tasks
@justinmk justinmk merged commit 3aa95ef into neovim:master Dec 4, 2019
@justinmk
Copy link
Member

justinmk commented Dec 4, 2019

The merge looks weird because of a GitHub bug: "Retry" button after "failed to merge" decided to do a merge-commit with wrong commit message. But it is a merge commit anyway, where @teto is the author of 2nd parent.

@teto teto deleted the nostartofline branch December 4, 2019 09:47
@teto
Copy link
Member Author

teto commented Dec 4, 2019

thanks ! I am bit nervous to how people will react about the change as it impacts a few bindings like gg/>> etc. Let's see :D

@zhoosch
Copy link

zhoosch commented Feb 1, 2020

I do not like changes in the defaults. Because i use vim and neovim with only one rc file. Even if someone changes the defaults i have to change these too in my configurations too, to have the same result in both editors. So my startup of vim/neovim eats more and more time.

So why do the ppl who like this new behavior do not change this one line in their init? set nostartofline. Too much effort?

@justinmk
Copy link
Member

justinmk commented Feb 1, 2020

Nvim defaults typically will save time since they are intended to be more commonly desirable.

And if you're worried about startup time then you should be worried about Vim's defaults.vim.

@neovim-discourse
Copy link

This pull request has been mentioned on Neovim Discourse. There might be relevant details there:

https://neovim.discourse.group/t/copying-original-vi-behaviour/1889/9

@neovim-discourse
Copy link

This pull request has been mentioned on Neovim Discourse. There might be relevant details there:

https://neovim.discourse.group/t/poll-should-startofline-option-be-on-or-off-by-default/1952/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defaults issues or PRs involving changing the defaults
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants