Generate links when diary & wiki directory are the same

[patstockwell]

🦒 Why

Generating links with the :VimwikiGenerateLinks command fails to add any links when the diary directory is the same as the wiki directory. When generating links, we first check that the file is not a diary file as we don't want to include those in the list. That work is delegated to the is_diary_file function. Prior to this change, that function always returned true if the file was in the diary directory. This approach gives false positives for a wiki which has a flat structure and the wiki files and diary files share a directory.

Sharing a directory is common when the wiki structure is flat and all files are at the same level. If diary_rel_path is set to './' then the root directory of the wiki will also be the diary directory.

✨ What

This change ensures that is_diary_file can check the given file name against an exact list of diary files. This results in :VimwikiGenerateLinks working correctly, even when the diary and the wiki share a directory.

Also:

• Improved path Normalization
• get_diary_files exported s:get_diary_files -> vimwiki#diary#get_diary_files

🌵 How

• Refactor vimwiki#base#is_diary_file to compare the given file against an exact list of diary files
  • Reuse an existing diary.vim function - vimwiki#diary#get_diary_files
  • Reuse an exisiting path.vim function - vimwiki#path#normalize
• Update vimwiki#path#normalize to normalise file paths that include a single /./. e.g. /Users/me/wiki/./diary/

🍭 Who

👋 Hi, my name is Pat. I'm a first-time contributor. Feedback welcome!

🖼️ Random diary GIF

📋 Checklist

• PR being made against the dev branch
• Have reviewed CONTRIBUTING.MD
• No related GitHub Issues
• Description of the proposed changes provided
• Documentation in doc/vimwiki.txt updated including the changelog and contributors sections.

[patstockwell]

Because we are in a loop here, vimwiki#base#is_diary_file is called for each file in the wiki. is_diary_file needs a list of diary files, which it gets by calling vimwiki#diary#get_diary_files(). This function also loops through all the files to check each against a matching diary file regex. That results in O(n²) operation.
Instead, we can get the diary files just once up front (line 394) and reuse that list inside the for loop. We pass the diary list as the optional second argument to is_diary_file.

[tinmarino]

Feedback: Excelent job. The best PR I've ever seen (I got only 4 years experience but still)
I would add:

• Better path Normalization
• get-diary-files exported : s: -> vimwiki#diary
  Perfect !

Now specifically:

• You remove ./ from path. shouldn't you transform /./ to / if I have a file ending with a dot ?
• I'll make the tests tomorrow, at least this week and well, this will soon be accepted.
• You did a hacky thing with getting a list of your diary files. I understand the intention but must have a look. Seems that you had O(number-of-wiki-and-diary-file) to GenerateLink because your case need to discriminate wiki from the diary. Is that it ? (PS: Ok I got it: you gzt the list here AND in is-diary-file). I think, it is too much CPU and IO for user that have a dedicated diary file.
  So, I know it add 5 lines of code, but I personaly prefer to add a check, see if we are obliged to work in wihich case call get-diary-files. in other word, hide the 2 calls to get-diary-file that you haded, so that my config (there are in diary), do not call it. What do you think ?

Once again, very good work. This movated me to get hands on a make the travis.yml (so pushers and maiteners can see if tests pass or fail)

[patstockwell]

Hi @tinmarino, thanks for your quick response. 🎉

---

I'll make the tests tomorrow, at least this week and well, this will soon be accepted.

Thanks! 🙏 I attempted to write some tests but couldn't run the test suite. Calling ./run_tests.sh gave me the error

Error response from daemon: pull access denied for vimwiki, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.

I'll need to investigate this a bit more.

---

You did a hacky thing with getting a list of your diary files. I understand the intention but must have a look. Seems that you had O(number-of-wiki-and-diary-file) to GenerateLink because your case need to discriminate wiki from the diary. Is that it ? (PS: Ok I got it: you gzt the list here AND in is-diary-file). I think, it is too much CPU and IO for user that have a dedicated diary file.

I see your point. However, checking for a diary directory first and using the existing method would still hide any files nested in the diary directory.

wiki
├── diary
│   ├── 2020-05-09.md <-- would not show 👍 
│   ├── diary_contacts
│   │   └── Bob.md <-- would not show 👎 
│   └── visited_cafes.md <-- would not show 👎 
├── index.md
└── stuff.md

Is this ok?

---

You remove ./ from path. shouldn't you transform /./ to / if I have a file ending with a dot?

Good pick up 🙌 I didn't consider directories ending with a .. Because the substituted value from the original regex is an empty string (/[^/]\+/\.\. -> ''), I'll need to create a second call to substitute that replaces with a slash (/./ -> /). Will update. 👍

[tinmarino]

Ok for the overhead.

Anyway locating all files is not so slow for less than 10.000 files and seems fair when re-indexing.
You forgot one renaming s:get_diary_file -> vimwiki#diary#get_diary_file so the mapping <leader>w<leader>i did complain. Fix in 00d2792 (just above your PR)

Conclusion: As the introduction, awesome work! Feel free to make other pull requests, big or small.

[patstockwell]

Ah, nice pick up. I appreciate the summary and the extra commit 👍
Thanks @tinmarino 🌮 🎉