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

[RDY] Remove POSIX 'cpoptions' #2943

Merged
8 commits merged into from Jul 19, 2015
Merged

[RDY] Remove POSIX 'cpoptions' #2943

8 commits merged into from Jul 19, 2015

Conversation

ghost
Copy link

@ghost ghost commented Jul 3, 2015

These were present for compatibility with the POSIX spec of vi.
Vi compatibility has slowly been stripped away in various forms (cpoptions, nocompatible), so the removal of these options seems to me like a logical continuation of that.

This is being done in order to fulfil #2548.

@ghost ghost added the compatibility compatibility with Vim or older Neovim label Jul 3, 2015
@marvim marvim added the RFC label Jul 3, 2015
@ghost ghost changed the title [RFC] Remove POSIX 'cpoptions' [WIP] Remove POSIX 'cpoptions' Jul 4, 2015
@ghost
Copy link
Author

ghost commented Jul 4, 2015

Setting this to WIP until I fix the tests.

@marvim marvim added WIP and removed RFC labels Jul 4, 2015
@ghost ghost changed the title [WIP] Remove POSIX 'cpoptions' [RFC] Remove POSIX 'cpoptions' Jul 6, 2015
@ghost
Copy link
Author

ghost commented Jul 6, 2015

Fixed the tests, so I'm setting back to RFC.

@marvim marvim added RFC and removed WIP labels Jul 6, 2015
@ghost
Copy link
Author

ghost commented Jul 6, 2015

The failure is on QB only, and looking through the log it looks to be related to QB itself, as opposed to our tests.

@ghost
Copy link
Author

ghost commented Jul 10, 2015

Rebased.

Would anyone like me to split this into multiple PRs?

@ghost
Copy link
Author

ghost commented Jul 19, 2015

rebased

@justinmk I can split this if you'd like, I imagine this might be tedious for people to review (although the commits are cleanly separated). Either way, I can't deal with vi_diff.txt without this.

@fmoralesc
Copy link
Contributor

Took a quick look, and this looks pretty nice.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

LGTM

@ghost ghost changed the title [RFC] Remove POSIX 'cpoptions' [RDY] Remove POSIX 'cpoptions' Jul 19, 2015
@ghost
Copy link
Author

ghost commented Jul 19, 2015

Great, thanks for the reviews. I'll merge soon if no more comments.

@marvim marvim added RDY and removed RFC labels Jul 19, 2015
@ghost
Copy link
Author

ghost commented Jul 19, 2015

I removed the sentence in recover.txt with the contents sets a flag in the current buffer, but that's it, spotted by @mhinz.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

Ah, the changes still have to be documented in vim_diff.txt.

@ghost
Copy link
Author

ghost commented Jul 19, 2015

Ah, the changes still have to be documented in vim_diff.txt.

Funny you say that, as I just pushed a commit for that 😄

A few cpoptions were removed before, so I documented those too. The line is pretty long, so I'm open to suggestions on how to format it differently.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

The line is pretty long, so I'm open to suggestions on how to format it differently.

Seems to fit perfectly! (Exactly 78 characters.)

(Although I find the prior removals of the non-POSIX flags from cpoptions to be quite controversial.)

@ghost
Copy link
Author

ghost commented Jul 19, 2015

(Although I find the prior removals of the non-POSIX flags from cpoptions to be quite controversial.)

I don't mind discussing this here as there's not much left to discuss regarding the PR, so why do you find them controversial? I went over a few of the PRs (the relevant commits all have cpoptions in the name) and they didn't seem useful.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

That's the problem. I can't know what other people find useful.. personally I think that g, w, - and j don't seem too useless to me. Actually I'm pretty sure I saw vimrcs using w and j before. Other flags might be more useful for plugins that temporarily need a certain behaviour.

IMHO, unless options are "vi settings" or there a good technical reasons for it, no option should be removed.

Using saner default values for new users is just as important as not shooing away long-time users by crippling their settings.

@ghost
Copy link
Author

ghost commented Jul 19, 2015

Sorry, I accidentally hit reply. Fully reply coming.

@ghost
Copy link
Author

ghost commented Jul 19, 2015

That's the problem. I can't know what other people find useful

Yes, I suppose you're right. Given that, I don't think we've gotten a single complaint so far, at least outside of Gitter (I didn't look through the logs).

personally I think that g, w, - and j don't seem too useless to me

I suppose I should be more precise. Of course they aren't useless, as that could be said about anything which isn't a no-op, but I still think they're close to it. I would really like to hear of some use cases for these commands, because I don't really understand why many of them exist besides Vi compatibility.

IMHO, unless options are "vi settings" or there a good technical reasons for it, no option should be removed.

The cpoptions flags removed so far have undoubtedly removed complexity in the surrounding code, but then again it hasn't been by much (in most cases).

Using saner default values for new users is just as important as not shooing away long-time users by crippling their settings.

Of course, but I'm not convinced we're shooing anyone away, as previously mentioned with the lack of complaints.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

Users won't complain, they will just continue using Vim, because it works for them. One could probably argue that it's not Neovim's intent to convert all existing Vim users, though.

Just my experience from #vim (IRC/Freenode):

There are many Vim users. Most heard of Neovim. Those who heard of Neovim, but haven't used it yet, think of it as a Vim fork that does things asynchronously and includes a terminal emulator (like Emacs). These are the killer features for them. Some people just want to play around with new software, then encounter a bug, and just go away. No feedback. Other people see cool plugins like neomake and neoterm. They are try to make them part of their workflow. Then they can't use <c-h>. Then colors are different. Then the editor behaves different (because settings they don't even know about were changed). Then they just go back to Vim and use vim-dispatch, which works just as good as neoterm and neomake. No feedback.

That's my experience.

People try out Neovim for the shiny things (and that doesn't include the great msgpack API), and leave because of small things that are different from Vim. Why? Because they think of Neovim as a superset of Vim. It's Vim + magic. But the truth is, currently Neovim does less than Vim.

People don't see that's it's all about the long term or that there isn't even a 0.1 version. There's nothing we can do about that, but we shouldn't make it any harder for them by changing things they expect.

Don't get me wrong, I think the project moves into the right direction, but then I'm more involved into Vim as most "normal users" who just want to get work done.

IMHO, we should focus on making all new stuff rock-solid, before thinking about changing options.

p.s. Sorry for the unstructured post, I just quickly wrote down what came to my mind. :)

@fmoralesc
Copy link
Contributor

IMHO, we should focus on making all new stuff rock-solid, before thinking about changing options.

Your rant would have given me pause when working on #2676, but then, I feel the issue is not really changing the defaults, but removing behaviours we might not get otherwise. I see now the point of keeping things like - or j in cpoptions (g and w, not so much). The POSIX compatibility options, however, seem to me of much more limited value: # limits the editing language grammar, & tries to mimic the backup functionality, \ can break regexes, / enables a more cumbersome idiom than s//.../ (this is pretty innocuous, though), '.' is not needed in Vim, | is already ineffective (the tui doesn't care about it). { can be useful in C-like languages, though, I can see people still using that.

@mhinz
Copy link
Member

mhinz commented Jul 19, 2015

Your rant

It was no rant.. maybe a bit. ;]

The POSIX compatibility options, however, seem to me of much more limited value.

Agreed. As I said in my post above my rant, removing all stuff that is somehow related to vi settings, should eventually removed.

@fmoralesc
Copy link
Contributor

It was no rant.. maybe a bit. ;]

I didn't mean it in a bad way ;)

Michael Reed added 8 commits July 19, 2015 15:14
It wasn't even hooked up to anything... must have been removed when
term.c was replaced.
- CPO_ALL and CPO_VI are identical, so merge them
- No longer check for the environment variable 'VIM_POSIX'
- In vim_diff.txt, mention the removal of 'cpoptions' flags
@ghost
Copy link
Author

ghost commented Jul 19, 2015

@mhinz

IMHO, we should focus on making all new stuff rock-solid, before thinking about changing options.

Yes, I think you're right (I read your rant and agree with it, so I don't have much else to say).

Now that I think about it, my main gripe with cpoptions--which I didn't say before--is that many of them effectively change core editing behavior. Of course we already have many options which change how nvim behaves on the surface, but not many which change how commands like :write and :read work, or how basic movements work. In general, I'm just no fan of having all these knobs which provide a seemingly negligible benefit, while complicating the implementation.

I guess I didn't mention it before because IMO it's impractical in the case of N/Vim, as the design philosophy is so different I think we're always going to have a tons of knobs present, although hopefully a bit less than vanilla Vim.

ghost pushed a commit that referenced this pull request Jul 19, 2015
[RDY] Remove POSIX 'cpoptions'

Reviewed-by: Felipe Morales <hel.sheep@gmail.com>
Reviewed-by: Marco Hinz <mh.codebro@gmail.com>
@ghost ghost merged commit 259db27 into neovim:master Jul 19, 2015
@jszakmeister jszakmeister removed the RDY label Jul 19, 2015
@ghost ghost deleted the rm-posix branch July 19, 2015 19:39
@ghost ghost mentioned this pull request Jul 19, 2015
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility compatibility with Vim or older Neovim
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants