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] Merge 'vimdiff' with 'nvim -d' #1849

Merged
merged 2 commits into from Feb 3, 2015

Conversation

Projects
None yet
7 participants
@ghost

ghost commented Jan 20, 2015

As discussed in #1646, this merges vimdiff with nvim -d. It's worth mentioning that vimdiff is the exact same thing as nvim -d (the former is just a symlink).

@marvim marvim added the RFC label Jan 20, 2015

@fmoralesc

This comment has been minimized.

Show comment
Hide comment
@fmoralesc

fmoralesc Jan 20, 2015

Contributor

👍

Contributor

fmoralesc commented Jan 20, 2015

👍

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jan 20, 2015

Member

I'd vote for keeping the symlink handling (#1646 (comment)), what do you think?

Member

fwalch commented Jan 20, 2015

I'd vote for keeping the symlink handling (#1646 (comment)), what do you think?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 20, 2015

I'd vote for removing it, for the reasons stated by @fmoralesc in the last post of that thread. As you said, a tiny shell script could be used instead. If we're going to remove duplicate interfaces from the docs, then we might as well remove their (limited) references in the code as well.

On January 20, 2015 5:15:09 AM EST, Florian Walch notifications@github.com wrote:

I'd vote for keeping the symlink handling
(#1646 (comment)),
what do you think?


Reply to this email directly or view it on GitHub:
#1849 (comment)

ghost commented Jan 20, 2015

I'd vote for removing it, for the reasons stated by @fmoralesc in the last post of that thread. As you said, a tiny shell script could be used instead. If we're going to remove duplicate interfaces from the docs, then we might as well remove their (limited) references in the code as well.

On January 20, 2015 5:15:09 AM EST, Florian Walch notifications@github.com wrote:

I'd vote for keeping the symlink handling
(#1646 (comment)),
what do you think?


Reply to this email directly or view it on GitHub:
#1849 (comment)

@ghost ghost closed this Jan 20, 2015

@jszakmeister jszakmeister removed the RFC label Jan 20, 2015

@ghost ghost reopened this Jan 20, 2015

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 20, 2015

Oops, sorry.

ghost commented Jan 20, 2015

Oops, sorry.

@marvim marvim added the RFC label Jan 20, 2015

@@ -57,7 +57,6 @@ static char *(features[]) = {
"+cursorbind",
"+cursorshape",
"+dialog_con",
"+diff",

This comment has been minimized.

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

I don't think this should be removed, some plugin code might want to check on has('diff').

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

I don't think this should be removed, some plugin code might want to check on has('diff').

This comment has been minimized.

@ghost

ghost Jan 20, 2015

This is solely for --version and :version output.

has('feature') is handled here https://github.com/neovim/neovim/blob/master/src/nvim/eval.c#L9892

@ghost

ghost Jan 20, 2015

This is solely for --version and :version output.

has('feature') is handled here https://github.com/neovim/neovim/blob/master/src/nvim/eval.c#L9892

This comment has been minimized.

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

Ah, OK. Please disregard.

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

Ah, OK. Please disregard.

This comment has been minimized.

@Hinidu

Hinidu Jan 20, 2015

Contributor

Actually there are plenty vim features which are non-optional in nvim, so perhaps we should do something with all such features in that list. I'm not sure that removing just that one is a good idea.

@Hinidu

Hinidu Jan 20, 2015

Contributor

Actually there are plenty vim features which are non-optional in nvim, so perhaps we should do something with all such features in that list. I'm not sure that removing just that one is a good idea.

This comment has been minimized.

@ghost

ghost Jan 20, 2015

@Hinidu I just wanted to keep this PR well contained, since I already removed all references in the docs to +diff. I do agree with you though, and I already have a feature branch for that here:
https://github.com/Pyrohh/neovim/commits/cleanup-version-output

The output might need some work though, as it looks a bit odd/incomplete:

NVIM 0.0.0-alpha+201501201544 (compiled Jan 20 2015 15:44:59)
Commit: 144c3fc0587ead7f51b9c02438f2a10e5747776c
Build type: Debug
Compilation: /usr/bin/cc -Wconversion -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -DINCLUDE_GENERATED_DECLARATIONS -DHAVE_CONFIG_H -I/home/michael/src/neovim/build/config -I/home/michael/src/neovim/src -I/home/michael/src/neovim/.deps/usr/include -I/home/michael/src/neovim/.deps/usr/include -I/home/michael/src/neovim/.deps/usr/include/luajit-2.0 -I/usr/include -I/home/michael/src/neovim/build/src/nvim/auto -I/home/michael/src/neovim/build/include
Compiled by michael@orchid  Features included (+) or not (-):
+acl      +iconv    +terminfo 
   system vimrc file: "$VIM/nvimrc"
     user vimrc file: "$HOME/.nvimrc"
 2nd user vimrc file: "~/.nvim/nvimrc"
      user exrc file: "$HOME/.exrc"
  fall-back for $VIM: "/usr/local/share/nvim"
@ghost

ghost Jan 20, 2015

@Hinidu I just wanted to keep this PR well contained, since I already removed all references in the docs to +diff. I do agree with you though, and I already have a feature branch for that here:
https://github.com/Pyrohh/neovim/commits/cleanup-version-output

The output might need some work though, as it looks a bit odd/incomplete:

NVIM 0.0.0-alpha+201501201544 (compiled Jan 20 2015 15:44:59)
Commit: 144c3fc0587ead7f51b9c02438f2a10e5747776c
Build type: Debug
Compilation: /usr/bin/cc -Wconversion -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -DINCLUDE_GENERATED_DECLARATIONS -DHAVE_CONFIG_H -I/home/michael/src/neovim/build/config -I/home/michael/src/neovim/src -I/home/michael/src/neovim/.deps/usr/include -I/home/michael/src/neovim/.deps/usr/include -I/home/michael/src/neovim/.deps/usr/include/luajit-2.0 -I/usr/include -I/home/michael/src/neovim/build/src/nvim/auto -I/home/michael/src/neovim/build/include
Compiled by michael@orchid  Features included (+) or not (-):
+acl      +iconv    +terminfo 
   system vimrc file: "$VIM/nvimrc"
     user vimrc file: "$HOME/.nvimrc"
 2nd user vimrc file: "~/.nvim/nvimrc"
      user exrc file: "$HOME/.exrc"
  fall-back for $VIM: "/usr/local/share/nvim"

This comment has been minimized.

@ghost

ghost Jan 20, 2015

Also worth mentioning is that I might have removed too many features from that branch, but my understanding is that platform parity is a big goal (hence the removal of the +mouse_* and X11 related items.

@ghost

ghost Jan 20, 2015

Also worth mentioning is that I might have removed too many features from that branch, but my understanding is that platform parity is a big goal (hence the removal of the +mouse_* and X11 related items.

This comment has been minimized.

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

Perhaps Features included (+) or not (-): could be changed for Includes support for:.

Also, a list of removed features would be good to have, to make clean how nvim differs from vim: -clipboard, -clientserver -cryptv, etc. Just as an high level overview, there is no point in being too detailed.

@fmoralesc

fmoralesc Jan 20, 2015

Contributor

Perhaps Features included (+) or not (-): could be changed for Includes support for:.

Also, a list of removed features would be good to have, to make clean how nvim differs from vim: -clipboard, -clientserver -cryptv, etc. Just as an high level overview, there is no point in being too detailed.

This comment has been minimized.

@ghost

ghost Jan 20, 2015

(I accidentally replied in the main thread)

@ghost

ghost Jan 20, 2015

(I accidentally replied in the main thread)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 20, 2015

I agree, assuming you're not talking about putting such a list in the --version output. I think a good idea would be to point users to something like runtime/doc/nvim_diff.txt via README.md and runtime/doc/nvim_intro.txt, so it could be viewed on Github and in nvim. I know the wiki page exists, but it's not very comprehensive.

Then again, we could just update the wiki page :)

ghost commented Jan 20, 2015

I agree, assuming you're not talking about putting such a list in the --version output. I think a good idea would be to point users to something like runtime/doc/nvim_diff.txt via README.md and runtime/doc/nvim_intro.txt, so it could be viewed on Github and in nvim. I know the wiki page exists, but it's not very comprehensive.

Then again, we could just update the wiki page :)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 21, 2015

Squashed the docs changes, rebased and repushed.

ghost commented Jan 21, 2015

Squashed the docs changes, rebased and repushed.

@justinmk

View changes

Show outdated Hide outdated runtime/doc/diff.txt Outdated
@Hinidu

This comment has been minimized.

Show comment
Hide comment
@Hinidu

Hinidu Jan 21, 2015

Contributor

I think a good idea would be to point users to something like runtime/doc/nvim_diff.txt via README.md and runtime/doc/nvim_intro.txt

I also thought that the good option is to remove this information from --version output, but provide it somewhere in the docs.

Contributor

Hinidu commented Jan 21, 2015

I think a good idea would be to point users to something like runtime/doc/nvim_diff.txt via README.md and runtime/doc/nvim_intro.txt

I also thought that the good option is to remove this information from --version output, but provide it somewhere in the docs.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 21, 2015

I also thought that the good option is to remove this information from --version output, but provide it somewhere in the docs.

Indeed. I might as well do that along with the --version cleanup, if only a stub.

ghost commented Jan 21, 2015

I also thought that the good option is to remove this information from --version output, but provide it somewhere in the docs.

Indeed. I might as well do that along with the --version cleanup, if only a stub.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 28, 2015

Marking as ready, feel free to remove it if I missed something.

ghost commented Jan 28, 2015

Marking as ready, feel free to remove it if I missed something.

@ghost ghost changed the title from [RFC] Merge 'vimdiff' with 'nvim -d' to [RDY] Merge 'vimdiff' with 'nvim -d' Jan 28, 2015

@marvim marvim added RDY and removed RFC labels Jan 28, 2015

Michael Reed
Remove vimdiff
While we're here:
- Remove references to the '+diff' feature, which has since been made
  non-optional.
- Update a few Vim instances with Nvim.
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 3, 2015

Rebased and squashed.

ghost commented Feb 3, 2015

Rebased and squashed.

@justinmk

View changes

Show outdated Hide outdated runtime/doc/diff.txt Outdated
@justinmk

View changes

Show outdated Hide outdated runtime/doc/diff.txt Outdated
@justinmk

View changes

Show outdated Hide outdated runtime/doc/usr_28.txt Outdated
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 3, 2015

Pushed the suggestions.

ghost commented Feb 3, 2015

Pushed the suggestions.

@justinmk justinmk merged commit a720b64 into neovim:master Feb 3, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@justinmk justinmk removed the RDY label Feb 3, 2015

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk Feb 3, 2015

Member

merged and made some small changes. Thanks!

Member

justinmk commented Feb 3, 2015

merged and made some small changes. Thanks!

@ghost ghost deleted the merge_nvim_-d_vimdiff branch Feb 4, 2015

@lucc

This comment has been minimized.

Show comment
Hide comment
@lucc

lucc Feb 9, 2015

Contributor

@Pyrohh You removed to much at this point. The two words "Unterschiede an." have to stay to form a correct German sentence.

Contributor

lucc commented on runtime/doc/manpages/de/vim-de.1 in 7b98b4c Feb 9, 2015

@Pyrohh You removed to much at this point. The two words "Unterschiede an." have to stay to form a correct German sentence.

@ghost ghost referenced this pull request Feb 21, 2015

Merged

Upcoming Newsletter #82

22 of 22 tasks complete

@ghost ghost referenced this pull request Mar 21, 2015

Merged

[RFC] Version output cleanup #2214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment