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

Fix uninitialized parameter dictionary #234

Conversation

Dudemullet
Copy link
Contributor

Adds a method that lets users define a single vdebug option without the need to create a dictionary in their .vimrc fixes #200 as vim users might try to add options that way ie. let g:vdebug_options['port']=10000.

How this code proposes to fix the issue is by still accepting the options dictionary (being backwards compatible) but by also accepting options with the format let g:vdebug_options_port, let g:vdebug_options_break_on_open.

I understand that with the inclusion of this method we would need to update the documentation, I'd be more than willing to do that if you believe this to be a good solution @joonty.

NOTE

In case this does get approved, it is important to note that paramters will have a different preference since we could have two different methods applying them at startup. basically
defaults < object method < single param method

So defaults get overwritten by the regular method of defining options. But if the same option gets set with the new method that overwrites the object method.

lets say we have this in a .vimrc

let g:vdebug_options = {}
let g:vdebug_options[port] = 8000
let g:vdebug_options_port = 8888

The configuration ends up being port = 8888. This behaviors precedence can be easily modified if we wanted to.

@joonty
Copy link
Collaborator

joonty commented Dec 22, 2015

Hi @Dudemullet, thanks very much for the work you put into this. Just to let you know, I've frozen all new development on versions 1.x.x of vdebug, in an effort to get vdebug 2 out. This would be a good opportunity to introduce your changes.

I've been thinking a lot about this, as the dictionary way of setting options has regularly caused problems for people in different ways. Thanks again, I'll get back to this when the code's in a good place to merge.

@joonty joonty added this to the Version 2.0.0 milestone Dec 22, 2015
@Dudemullet
Copy link
Contributor Author

Sure @joonty, no problem. If you want I could merge to 2.0 branch or re-create the pr once 2.0 is in master. Your call.

Also, I tried developing some tests for this but this is my first experience with vimscript development and couldn't find an existing test suite for that particular section of the code. I'd gladly also add the tests if/when you decide on a testing platform for the vimscript portion of the code. 👍 Thanks again

@josx
Copy link

josx commented Jan 19, 2016

👍 to merge this!

@BlackIkeEagle BlackIkeEagle added this to TODO in v2.0 Mar 23, 2017
@BlackIkeEagle BlackIkeEagle moved this from TODO to In Progress in v2.0 Mar 23, 2017
@BlackIkeEagle
Copy link
Member

@Dudemullet If you are still willing to write the documentation for this it would be great

I have merged this in v2-integration for testing

@Dudemullet
Copy link
Contributor Author

@BlackIkeEagle Sure, on it 👍🏼

@BlackIkeEagle
Copy link
Member

@Dudemullet thank you very much 👍

Removes all instances in the documentation where plugin configuration
was made via Dictionary key modification.
@Dudemullet
Copy link
Contributor Author

Hmm looks like I need to rebase. I ran the local tests and everything passed fine or is this error ocurring because of something else ?

The command "pip install -r requirements.txt --use-mirrors" failed and exited with 2 during

@BlackIkeEagle
Copy link
Member

@Dudemullet that is already fixed in v2-integration

thanks for the update, I'll probably not be able to continue the integration this week since I'm moving houses. Probably next week there will be more activity again :)

@BlackIkeEagle
Copy link
Member

@Dudemullet Thanks I added your documentation update to the v2-integration branch

@BlackIkeEagle BlackIkeEagle moved this from In Progress to Done in v2.0 Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v2.0
Done
Development

Successfully merging this pull request may close these issues.

Can't set g:vdebug_options from .vimrc
4 participants