MarcWeber / vim-addon-manager

manage and install vim plugins (including their dependencies) in a sane way. If you have any trouble contact me. Usually I reply within 24 hours
Other
660 stars 59 forks source link

doc: fix error when refusing to clone VAM #75

Closed vivien closed 12 years ago

vivien commented 12 years ago

With the setup described in the VAM-installation documentation, saying no when asking to clone VAM (very first question) will result in:

$ vim

Clone VAM into /home/vivien/.vim/vim-addons?
[Y], (N): n

Error detected while processing function SetupVAM:
line   15:
E117: Unknown function: vam#ActivateAddons
Press ENTER or type command to continue

This commit improves the EnsureVamIsOnDisk() function to fix this issue.

Signed-off-by: Vivien Didelot vivien@didelot.org

MarcWeber commented 12 years ago

I'm not sure whether I want that patch because its adding error handling. The setup code should be as simple as possible. The main question is: Do you want a way to start vim without VAM? If yes then it would be enough to only call vam#Activate if the directory exists (that would be adding a if file_readable() ... endif only (2 additional lines) - thus much shorter. The most important question is: How much is going to break if you don't load your plugins - does that one error message matter much? The setup code is a sample only - thus you can adjust it to your needs anyway.

vivien commented 12 years ago

I agree with not making the setup code complicated, that's why it only adds 6 lines. The main reason why I wrote this patch is, I don't think you should have vim errors with this sample code, especially when you simply reply no to the question. What do you think?

MarcWeber commented 12 years ago

Excerpts from Vivien Didelot's message of Mon Jun 04 02:11:12 +0200 2012:

I agree with not making the setup code complicated, that's why it only adds 6 lines. The main reason why I wrote this patch is, I don't think you should have vim errors with this sample code,

I don't care. I'm waiting for others to vote for this feature. Your version is too complicated. If I add it I add it as

if file_readable(vam_path..'/.git') call vam# ... endif

Because then adding 2 lines is enough - and you don't have to introduce cryptic return numbers. Maybe even assigning a global g:dont_have_vam is the way to go - because then the user can use that global var in multiple places - but again I consider this being ovenkill. Either you want VAM - or you don't.

vivien commented 12 years ago

Either you want VAM - or you don't

Then why does the setup ask for confirmation to clone VAM? If you don't want confirmation, just get rid of line 124.

Anyway, I just wanted to help, avoiding possible errors from the doc. Feel free to reject it.

Thanks, Vivien.

MarcWeber commented 12 years ago

fixed in 919a9d. Note that I recheck the condition after the shell command has been run. I know eval is evil - but this is the best way to avoid duplicate code :(