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

Do not modify spaces to link descriptions #1132

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

vinnyfuria
Copy link
Contributor

@vinnyfuria vinnyfuria commented Jun 2, 2021

When links_space_char is configure to replace spaces, the spaces should only replace the __LinkUrl__ and not replace the __LinkDescription__

This is a different fix then described in #1038 but fixes the described problem.

All vadar tests pass; vint returns no errors

Note: on some vim versions I'm seeing 1/4 tests passing in search.vader; but that is the case in the dev branch as well.

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.

@vinnyfuria
Copy link
Contributor Author

I'm not sure how to craft a vadar test for this change; assistance or advice for doing so would be appreciated.

@tinmarino
Copy link
Member

tinmarino commented Jun 3, 2021

Hi @vinnyfuria,
Tk for the PR, it seems nice from a glimpse and a Vador test would be nice thank you also for proposing it (it is really less job to integrate, maintain).

You can:
1/ Paste here some in / out, I mean formalize the test
2/ Use a private fork of vimwiki and puch on this branch so the .travis executes.
3/ Run the vimwiki/test/run_test.sh script, it requires linux and docker (even if I tied to windowsify it a little)

There is a Readme in the /test, in local Vim use the :Vader command (and Vim plugin). And for the syntax, copy-paste other tests.

In 2h, sure you are done (30 min maybe). But it may be interesting.

@vinnyfuria
Copy link
Contributor Author

Thanks @tinmarino; I'll do my best:

  1. To test this change, links_space_char should be set to _ and syntax should be markdown.
    The test input text should be any text that contains spaces, for instance:
This is a test

Then, in visual mode, the string should be highlighted and then converted to a link by pushing <enter>. The result should be:

[This is a test](This_is_a_test)
  1. I'll push an update that includes the PR number in the changelog, hopefully that'll be enough to trigger Jenkins

  2. Output from the run_test.sh script (forgive the ugliness, I used script to capture the output): https://gist.github.com/256e6eb8d9dd280f0a2338c249d78044. As noted in the description, I'm seeing a few test failures in a subset of the versions; but the same test failures are occuring on the dev branch without my changes.

Add correct PR number
@vinnyfuria
Copy link
Contributor Author

I've gotten the outlines of a Vader test:

Execute (Log):
  Log 'Markdown with links_space_char set'

Given vimwiki (abc def ghi jkl):
  abc def ghi jkl

Execute (Configure links_space_char for test):
  call SetSyntax('markdown')
  let vimwiki_list_save = g:vimwiki_list
  let g:vimwiki_list = [{'path': '~/wiki/', 'syntax': 'markdown', 'ext': '.md', 'links_space_char': '_'}]

Do (vee<CR>):
  vee\<CR>

Expect (underscores in link url not in description):
  [abc def](abc_def) ghi jkl

Execute (restore):
  let g:vimwiki_list = vimwiki_list_save

However, setting the links_space_char in vimwiki_list doesn't seem to be taking effect (and I can reproduce this behavior in "normal" vim). Is there are command to force the reload of the configuration from the vimwiki_list? I don't think such a command exists; so I'm not sure how to integrate this test into Vader without breaking all the other tests.

@tinmarino tinmarino merged commit 725709b into vimwiki:dev Jun 8, 2021
tinmarino added a commit that referenced this pull request Jun 8, 2021
@tinmarino
Copy link
Member

Tk @vinnyfuria I just added the tests, It is always a hack to modify vimwiki variables (and restore peace at end).
This is why I did it, after 10 quick and dirty try, the 11th worked.

Thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants