Don't flag name duplication when installing the same plugin twice

[jdevera]

Name duplication is checked so that users don't end up overwriting one
plugin directory with the contents from another plugin that shares a
common name.

However, if the repeated plugin happens to be the same, there will be no
surprises and there is no need to flag this as an error. This needs if
particularly acute due to the fact that we currently don't show, in the
plugin search, which of the resulting plugins are already installed, so a
user might very well attempt to install a plugin they already have.

With this change, instead of seeing an error in this scenario, the user
will see a message indicating the plugin is already installed, and no
additional cloning will be performed.

[jdevera]

@lucc @Av3r4ge @gmarik can any of you take a look? objections?

[annagrram]

Vimscript, not shellscript ;)

[annagrram]

Like you mentioned in #509:

However, when looking at the condition itself I think it would be more solid to compare the resolved URIs for both instead of just the specs, since Align and vim-scripts/Align are still the same Plugin, despite their different specs.

You can also have the same plugin with the same name but different URIs (e.g. Original Github repo vs. vim-script). Do you want to take this into account as well?

Currently taking it for a test drive. I will comment afterwards.

[annagrram]

Seems to work well except for a couple of problems.
One problem I've found is duplications in the :PluginList after an installation attempt of the same plugin.
Second problem is installing something from search when you already have this plugin from the original github repo. I get an error, but the remote of the local repo is overridden to vim-script.

I'm not sure if it's best to try handling name collisions for a single case of same name and same URI, and fail for the rest of the cases, or echoing an instructive message which asks the user to recheck the installed plugins. This can be done with a combination of :echohl and :echomsg instead of :echoerr, so the result will be more appealing and less frightening.

[jdevera]

Wow! that fi left my mind now in a deep state of W.T.F.. I clearly did not run the right tests. Thank goodness for pull requests and reviews :)

For the URL topic. I consider different URLs to be different plugins even when they conceptually are the same plugin. Since updates will be different and it will bring nasty surprises soon.

You raise a very good point with the remote change. This needs to be addressed for sure. I'll try to do that in the same push. I'm thinking of splitting name check and registration into two steps, with the name check first, and only if that passes, then we do the git operation, and after that the name registration. This way if we find it to be a duplicate we will not have changed anything in the repository.

I'll look into the lesss frightening options for showing messages too.

All in all, thank you very much @Av3r4ge for this thorough review.

[annagrram]

Glad to help.

Everyone has their W.T.F bugs, it's part of the fun.

For the URL topic. I consider different URLs to be different plugins even when they conceptually are the same plugin. Since updates will be different and it will bring nasty surprises soon.

Very well then. In that case this PR is great.
After fixing the two issues I raised, I'll take it into a second test drive.

[jdevera]

Ok, I made so that the unmentionable mistake never happened :P

I also reversed the order of operations when installing interactively so that errors with name will exit early and not cause the remote change you experimented.

Also made errors be warnings when they were more clearly user caused, and they did become less scary when used echomsg instead of echoerr.

[annagrram]

Looks great! The remote doesn't change and I don't get a scary error on conflict.
But, I still have two minor issues:

1. When installing the same plugin again via interactive mode, or just accidentally putting it twice in .vimrc, Vundle loads it n times. Meaning, you will find duplications in :PluginList and in rtp. Maybe just ignore a duplicate when verifying the name?
2. When I try to install a conflict, the error message says: "Error processing [plugin_name]", and the reason is suppressed (but can be found in :messages). Adding some explanation text to the error saying where you might find the reason can be beneficial, in my opinion.

[jdevera]

What do you mean by "when I try to install a conflict"?

[annagrram]

Installing a plugin with the same name but with a different URI.

[clausreinke]

Not sure whether you saw the commit notice (clausreinke@908662e) in the old issue, but to avoid duplicate identical entries I switched check_bundle_name to return 3 possible values, where the context would only proceed if the name was new.

[jdevera]

Closing this as I have lost the context here completely.