Oni: Add 'surround.vim' as a default plugin

[bryphe]

From discussion on discord, I believe it makes sense to include surround.vim in our set of default plugins.

A few things needed here:

• Add vim-surround as a submodule in vim\default : https://github.com/tpope/vim-surround
• Add an attribution and link in the Bundled Plugins section in our README.md

[badosu]

Do you think there's a way we can expose an option to not load the bundled plugins? It would be good if instead of it being on or off for all plugins we had a hook on activate or a new one (e.g. setup) that specifies the bundled plugins with the default ones explicit on the default config.js.

Another idea is to not expose on config.js but if the hook is defined to override the default one, so this way we achieve 2 good things: users can have an very straightforward package bundler for when you don't want to inherit your init.vim or use NeoBundle, Plug, etc..; and we allow for minimalists to have a way to not load too much cruft.

E.G.

const activate = (oni) => {
  oni.plugins.load("wellle/targets.vim");
  oni.plugins.load("tpope/vim-commentary");
  oni.plugins.load("tpope/vim-unimpaired");
  // ...
};

[bryphe]

@badosu - this is great feedback! I agree, I think this would be nicer than the oni.useDefaultPlugins option we have, as it is much more explicit. IMO It's not really clear how the oni.useDefaultPlugins behaves, what the ramifications of changing the value are, etc, and it changes if we decide to bundle a new plugin...

Actually, this is quite a bit in-line with what I'm starting to do with the plugins configuration option here in #1903 - the idea is you'd have a setting like:

"plugins": [
  "welle/targets.vim",
  "tpope/vim-commentary",
  "tpope/vim-unimpaired",
  "oni-plugin-live-preview",
  "oni-plugin-power-mode",
   etc...
]

The setting is actually there in master, but it is not fully implemented yet, so I don't recommend using it (yet). But I think semantically it is close, and with some relatively minor tweaks I believe it could fill this use case.

Another idea is to not expose on config.js but if the hook is defined to override the default one, so this way we achieve 2 good things: users can have an very straightforward package bundler for when you don't want to inherit your init.vim or use NeoBundle, Plug, etc..; and we allow for minimalists to have a way to not load too much cruft.

Agree 100%. What do you think of using the plugins configuration setting for this? By default, we could populate it with our initial set of plugins, but the user could always scrap some.

The other consideration I had was deciding when we should check for updates for plugins. I was thinking about having an API call, like: Oni.plugins.checkForUpdates() that could be run in the activate method. Do you have any ideas there?

My hope is that we could deprecate the oni.useDefaultConfig configuration setting once we both complete the work in #973, and we have a more explicit model for managing the built-in plugins (and plugins in general!)

[parkerault]

@bryphe as far as syntax goes, I prefer the array; less typing is good! For installation and updates I would prefer to have an explicit install and update function available in the config and also in the command palette; this is how Plug works:

[badosu]

@parkerault @bryphe Those are good ideas.

One improvement on that would be to provide a mechanism to specify a locked plugin, by denoting the ref for a plugin. This way we can choose locked versions for the bundled plugins to avoid nasty surprises and also provide flexibility for users.

A way to do it could be by specifying a hash, e.g.:

"plugins": [
  "tpope/vim-commentary",
  "https://gitlab.com/a/repo-vim",
  ["welle/targets.vim", { ref: 'abranch' }],
  ["tpope/vim-unimpaired", { ref: 'ac04411' }],
   // etc...
]

Also can be done explicitly (allows for different fetching mechanisms and easier to know what's really doing):

"plugins": [
  { github: "tpope/vim-commentary" },
  { git: "https://gitlab.com/a/repo-vim" },
  { github: "welle/targets.vim", ref: 'abranch' },
  { github: "tpope/vim-unimpaired", ref: 'ac04411' },
   // etc...
]

It would be good if we could leverage one of the already existing libraries to do that so that we don't have to maintain this complexity, but I am not sure there exists one that does not pollute the vim environment for the user.

[parkerault]

That reminds me, there are some plugins (vim-prettier off the top of my head) that require post installation setup; Plug handles this with a do block: Plug 'prettier/vim-prettier', { 'do': 'yarn install' }.

[joereynolds]

Hi!

Can I ask why you've chosen surround.vim over other alternatives? :)

As far as I'm aware, there are a few different attempts at 'surrounds' in vim so I want to know why Tim Pope's was chosen (aside from the fact that he writes some good plugins).

I have a personal preference towards for vim-sandwich? (feels more like the "vim way", has tests, more functionality) and I think if more people were aware of these alternatives, they might get a better shot at the limelight :smile:

Anyway, I guess I'm trying to say it's hard to please all the people all of the time and it's especially difficult when there's a bundled plugin that mimics something you're already using.

Thanks!

[badosu]

@joereynolds So I think that's a good example why we need to provide users a way to fine grain the plugin loading mechanism.

We can track the feature in #2020.

[CrossR]

Since there is #2020 for the ongoing discussion, should we close this out now that @Muream has added this?

[bryphe]

Agree, we can track the remaining issues in #2020 - like being more explicit about what is bundled / making it easy to opt-out / etc.

Thanks @CrossR , will close this out!