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

Allow hyphens instead of spaces and search case-insensitive in jump_to_anchor function #840

Closed
wants to merge 2 commits into from

Conversation

bratekarate
Copy link
Contributor

@bratekarate bratekarate commented Mar 25, 2020

Steps for submitting a pull request:
Disclaimer: Log is kind of messed up because I maintain my own fork and needed to pull the latest version in to my repo

  • 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.

This PR allows GFM style links to be used (Issue #831). jump_to_anchor has been altered so that search for anchors is now case-insensitive and dashes may be included instead of whitespaces.

Code works as follows: before the anchor is transformed into the regex patterns, a \c is prepended before the first character of each word and spaces are replaced by [ -]. This way the search function also matches GFM style links.

I'm not a vimscript expert, so I don't know if this solution is the prettiest, but it is a one liner. Feel free to improve if I missed something or if it's too hacky.

@bratekarate
Copy link
Contributor Author

bratekarate commented Mar 25, 2020

Fixed messed up log. That's why commit date changed to 15 minutes ago.

@ranebrown
Copy link
Contributor

Although this implementation partially works I think there are some cases that might be missed. See this comment of #666.

Things like VimwikiTOC and tags within two lines of a header for VimwikiGenerateTagLinks should be supported as well.

@bratekarate
Copy link
Contributor Author

bratekarate commented Apr 2, 2020

I'm not sure I understand why VimwikiTOC has something to do with this.

The Issue was not intended to be about the VimwikiTOC generating GFM compatible markdown, but rather to make vimwiki compatible with GFM style anchors. This implementation works with both VimwikiTOC anchors and GFM style anchors. At least I haven't found a VimwikiTOC generated anchor that did not work for me.

Are you saying the generation with VimwikiTOC should be modified in the same issue? Allowing vimwiki to recognize GFM style anchors -- what I did here -- and generating a valid GFM style TOC sounds like two different issues to me.

As for tags within two lines of a header, I am not too familiar with this functionality yet. Do you mean that tags in within two lines of a header can be referenced by anchors?

Edit: Maybe with cases that might be missed you refer to what is explained in this comment. I could make special characters work as well. Numberings for duplicates could be ignored to always jump to the first occurence. That is how the current TOC anchors work too.

@ranebrown
Copy link
Contributor

Are you saying the generation with VimwikiTOC should be modified in the same issue?

It would be nice if auto generated links to headers used the GFM format but I'm ok with that being a separate issue if you don't want to add it to this one.

The bigger concern is making sure all headers are properly supported such as ones with special characters.

Numberings for duplicates could be ignored to always jump to the first occurence.

Sounds reasonable.

@bratekarate
Copy link
Contributor Author

It would be nice if auto generated links to headers used the GFM format but I'm ok with that being a separate issue if you don't want to add it to this one.

I'm afraid I don't have the time right now. I may be able in the future, but I am not sure yet. Hence I would like to keep this a separate issue that can be resolved soon. This way it is possible to e.g. copy existing GFM style markdown in vimwiki and navigate the anchors.

The bigger concern is making sure all headers are properly supported such as ones with special characters.

That's clear. I will improve the code.

I just tried using links to tags within 2 lines of a heading with VimwikiGenerateTagLinks, it still works with this PR. Of course only within vim, not in GFM Markdown. This would have to be reworked together with VimwikiTOC

@ranebrown
Copy link
Contributor

Probably makes sense to create a function in autoload/vimwiki/u.vim to handle the link transformation.

@tinmarino tinmarino added the change-required Pull request need some changes from autor to be accepted label May 12, 2020
@YangVincent
Copy link

How can I get this working in the meantime? I've made the same change in my autoload/vimwiki/base.vim, but it doesn't seem to trigger on "enter".

@YangVincent
Copy link

Update -- I ended up adding
let fname = tolower(substitute(fname, ' ', '-', 'g')) on line 845 in vimwiki#base#edit_file, in base.vim.

@tinmarino tinmarino assigned tinmarino and unassigned tinmarino Jul 22, 2020
@tinmarino
Copy link
Member

tinmarino commented Jul 22, 2020

let fname = tolower(substitute(fname, ' ', '-', 'g')) on line 845 in vimwiki#base#edit_file, in base.vim.

This is very bad. You got cannot jump to files with Uppercase or spaces

let segment = substitute(substitute(segment, '\<\(.\)', '\\c\1', 'g'), '-', '[ -]', 'g')

This seems a possible start.
A little hacky really ? Much too much for me ! See in the PR (Edit: Oups we are in the PR ...)

@@ -659,6 +659,10 @@ function! s:jump_to_anchor(anchor) abort

for segment in segments

" prepare segment pattern so that it is case insensitive and also matches dashes
" in anchor link with spaces in heading
let segment = substitute(substitute(segment, '\<\(.\)', '\\c\1', 'g'), '-', '[ -]', 'g')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why one liner ?

" Craft Segment regexp
" Ignore case
let segment = substitute(segment, '\<\(.\)', '\\c\1', 'g')
" Treat - as [- or space]
let segment = substitute(segment , '-', '[ -]', 'g')

@tinmarino
Copy link
Member

From the famous comment: still missing the opposite of:

  1. remove anything that is not a letter, number, space or hyphen (see the source for how Unicode is handled)
  2. If that is not unique, add "-1", "-2", "-3",... to make it unique

@tinmarino
Copy link
Member

So what must be done before using the anchor:

  • Change the one liner way to something more readable (please) (5 min)
  • Add the letter, number, space or hyphen potential insertion anywhere (point 2) (15 min)
  • Add the -1 potential removal (point 4) (5 min)
  • Add some non regression tests (2h for the first time, 15 min afterwhile)

@bratekarate can you do that ? I can take the non regression test if you dont want to install the docker

@tinmarino tinmarino added the test-needed Add a non-regression test label Jul 22, 2020
@tinmarino tinmarino mentioned this pull request Jul 30, 2020
@tinmarino tinmarino closed this Jul 30, 2020
tinmarino added a commit to tinmarino/vimwiki that referenced this pull request Jul 30, 2020
…#840 from @bratekarate)

Brief: Permits:

```
[Link to heading](#heading-example)
```

Issue: Feature Request: Support GFM anchor links inside wiki files vimwiki#831
Original PR: Allow hyphens instead of spaces and search case-insensitive in jump_to_anchor function vimwiki#840
Author: @bratekarate (implementation) @tinmarino (test)

Related: vimwiki#666
@tinmarino
Copy link
Member

FIxed: 355c1f7
I made the modification (unhack the hacky) line and added test in the commit referencing this issue on dev

@bratekarate Thank you very much for the collaboration.
Feel free to make other small commits like that, if you describe the tests as well as in (Issue #831), I'll write the tests and integrate faster.

Some related changes can be found at #666

@tinmarino tinmarino removed change-required Pull request need some changes from autor to be accepted test-needed Add a non-regression test labels Jul 30, 2020
@bratekarate
Copy link
Contributor Author

@tinmarino Thank you so much for for completing this fix! No need to thank me, I punched in a one liner and then pretty much ghosted this PR whilst saying I would continue to work on it. Your test cases will probably help me better understand the testing framework in use.

I'm currently in the final stage of my studies at university and can't find any time for github projects in my free time. That is also why I did not respond. I look forward to further contributing to vimwiki after I finally finish my studies, as I still use vimwiki daily at work and privately. Without it, I probably would have to switch to Doom emacs and use org mode.

Also, sorry for necrobumping this PR with non-technical boilerplate text :)

@tinmarino
Copy link
Member

Hi @bratekarate,

Happy to see some people with the will to work. Don't worry, the project can wait for you. Once you feel ready: you have some time and want to code, better than making flappy bird, try to fix one or two good first issues.
It is now a Github convention to use this label and leave some "not so hard" stuff for easy joining. We would be very glad if you could join our ranks (See #624 for the entry door I used 3 month ago).

Without it, I probably would have to switch to Doom emacs and use org mode.

Same here. Plus Emacs has the advantage that it is not What You See is What You Get but the big drawback that it is never on my machine

Also, sorry for necrobumping this PR with non-technical boilerplate text :)

I forgive you, if you forgive me 🐻. In reality, it is always motivating to see some happy users and much more to treat with some potential contributors (Imagine if would be 10 for 2 months on Vimwiki: even Emacs people would come)

@bratekarate
Copy link
Contributor Author

Glad to hear that there are "not so hard" things to fix, since that probably fits my almost nonexistent vimscript skills. I will check out some of these said labelled issues as soon as I find time. 👍

deepredsky pushed a commit to deepredsky/vimwiki that referenced this pull request Jan 16, 2021
…#840 from @bratekarate)

Brief: Permits:

```
[Link to heading](#heading-example)
```

Issue: Feature Request: Support GFM anchor links inside wiki files vimwiki#831
Original PR: Allow hyphens instead of spaces and search case-insensitive in jump_to_anchor function vimwiki#840
Author: @bratekarate (implementation) @tinmarino (test)

Related: vimwiki#666
jls83 pushed a commit to jls83/vimwiki that referenced this pull request Jan 17, 2023
…#840 from @bratekarate)

Brief: Permits:

```
[Link to heading](#heading-example)
```

Issue: Feature Request: Support GFM anchor links inside wiki files vimwiki#831
Original PR: Allow hyphens instead of spaces and search case-insensitive in jump_to_anchor function vimwiki#840
Author: @bratekarate (implementation) @tinmarino (test)

Related: vimwiki#666
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

4 participants