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

Add option g:vimwiki_commentstring to disable vimwiki commentstring #946

Merged
merged 4 commits into from
Jul 30, 2020

Conversation

BirgerNi
Copy link
Contributor

Steps for submitting a pull request:

  • ALL pull requests should be made against the dev branch!
  • Take a look at CONTRIBUTING.MD
  • Reference any related issues.
  • Provide a description of the proposed changes.
  • PRs must pass Vint tests and add new Vader tests as applicable.
  • Make sure to update the documentation in doc/vimwiki.txt if applicable,
    including the Changelog and Contributors sections.

By default the commentstring ist set to %%%s. For markdown documents which should be exported to html via pandoc a commentstring like <!--\ %s\ --> may be more suitable. This PR provides the option g:vimwiki_commentstring to turn off vimwiki commentstring. The option ist turned on by default to mainain backwards compatibility.

@tinmarino
Copy link
Member

Nice, I did not know this feature.

So you let g:vimwiki_commentstring = 0 and set commentstring='<!-- %s -->
You think about what I think ? Why don't you just let g:vimwiki_commentstring = '<!-- %s -->' and null string to prevent vimwiki setting the vim var and unset to keep default behavior (which I dont like by the way).
In other words, let the option give the chance for the user to add some customization.
Would you mind adding some 3 regression tests ?

All that said, thank you for the contribution.

@tinmarino tinmarino added config Related to configuration: default, customization test-needed Add a non-regression test labels Jul 22, 2020
@tinmarino tinmarino self-requested a review July 22, 2020 05:15
@BirgerNi
Copy link
Contributor Author

Actually for me commentstring is set by vim-pandoc plugin so I'm just looking for a way to turn it off :-)

Anyway, you mean something like this:

if !exists('g:vimwiki_commentstring')
    setlocal commentstring=%%%s
elseif g:vimwiki_commentstring != ""                                                                                                                       
    setlocal commentstring=g:vimwiki_commentstring
endif

Unfortunately with let g:vimwiki_commentstring='<!-- %s -->' in my .vimrc I'm stuck with this error:

E537: 'commentstring' must be empty or contain %s

Do you have an idea?

@tinmarino
Copy link
Member

tinmarino commented Jul 27, 2020

Do you have an idea?

Yes (sorry for the delay)
setlocal commentstring, like any set command, should not get a variable like g:vimwiki_commentstring but a verbatim string like <!-- %s --> (without the quotes)

So embed it in an execute command

if !exists('g:vimwiki_commentstring')
    setlocal commentstring=%%%s
elseif g:vimwiki_commentstring != ""                                                                                                                       
    execute 'setlocal commentstring=' . g:vimwiki_commentstring   " <----- Replace this line
endif

will work

PS: Also you can (recomended):

let &l:commentstring=g:vimwiki_commentstring

which set the local values without the overshot of execute

@tinmarino
Copy link
Member

Then please:

  1. Add test
  2. Rebase

@tinmarino
Copy link
Member

It's perfect !
Sorry for the travis fails: my bad: I change the test format, I'll reformat your test.

@tinmarino tinmarino merged commit 0a5a33a into vimwiki:dev Jul 30, 2020
@tinmarino
Copy link
Member

I changed a little bit the code: Commit: 5651e74

  1. We have a standardized way to declare variables (so they are all in the same place): have a look at vars.vim; 'commentstring': {'type': type(''), 'default': '%%%s'},
  2. We do not use Include: vader_includes/vader_setup.vader

Your feature is added in the dev branch, good job, see you in an other PR

deepredsky pushed a commit to deepredsky/vimwiki that referenced this pull request Jan 16, 2021
jls83 pushed a commit to jls83/vimwiki that referenced this pull request Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Related to configuration: default, customization test-needed Add a non-regression test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants