WIP: document link resolution behavior and design decisions

This commit is starting as attempt to add useful context for link functions
in the comments / docstrings, and clean up a few things for better
readability.  This is preliminary work for a solution to:

https://github.com/vimwiki/vimwiki/issues/1271

...which is gradually becoming a description of everything that's wrong
with how VimWiki links to files, more or less.

It includes a decision document, which should become a full description
of the problem and steps taken.

This commit _will_ be rewritten.

Bug: #1271

autoload/vimwiki/base.vim

@@ -125,10 +125,21 @@ endfunction
 
 function! vimwiki#base#resolve_link(link_text, ...) abort
   " Extract infos about the target from a link.
-  " THE central function of Vimwiki.
-  " If the second parameter is present, which should be an absolute file path, it
-  " is assumed that the link appears in that file. Without it, the current file
-  " is used.
+  "
+  " THE central function of Vimwiki.  Here be dragons!
+  "
+  " This has become confusing and extremely brittle.  Don't change it unless
+  " you know exactly what you're doing and test every code path.  It is safe
+  " to assume that at least one user relies on everything this does, whether
+  " it was intentional or not.  Do not make breaking changes here, or you will
+  " hear about it.  If you want to add extra link syntax or behavior, please
+  " consult with other maintainers first!
+  "
+  " See doc/specification.wiki for a draft spec that covers link syntax.
+
+  " a:0 is set to the number of extra arguments.  If the second parameter is
+  " present, which should be an absolute file path, it is assumed that the
+  " link appears in that file. Without it, the current file is used.
   if a:0
     let source_wiki = vimwiki#base#find_wiki(a:1)
     let source_file = a:1
@@ -298,8 +309,20 @@ endfunction
 
 
 function! vimwiki#base#system_open_link(url) abort
-  " Open Link with OS handler (like gx)
-  " handlers
+  " Open Link with OS handler (like gx).
+  "
+  " This handles all supported URI-like links, including http:, file:, etc.,
+  " unless a custom VimwikiLinkHandler() does something with them first.
+  "
+  " It can be a source of OS-specific bugs, since behavior differs across
+  " operating systems and this makes some fairly basic assumptions about
+  " what's available.
+  "
+  " TODO: The easiest way for a user to change this behavior at present is to
+  " write a VimwikiLinkHandler().  This is probably fine, but there might be a
+  " use case for being able to explicitly set a handler just for system links.
+
+  " Handlers
   function! s:win32_handler(url) abort
     "Disable shellslash for cmd and command.com, but enable for all other shells
     "See Issue #560
@@ -329,12 +352,15 @@ function! vimwiki#base#system_open_link(url) abort
 
     endif
   endfunction
+
   function! s:macunix_handler(url) abort
     call system('open ' . shellescape(a:url).' &')
   endfunction
+
   function! s:linux_handler(url) abort
     call system('xdg-open ' . shellescape(a:url).' >/dev/null 2>&1 &')
   endfunction
+
   try
     if vimwiki#u#is_windows()
       call s:win32_handler(a:url)
@@ -449,7 +475,7 @@ function! vimwiki#base#generate_links(create, ...) abort
     let use_caption = vimwiki#vars#get_wikilocal('generated_links_caption', wiki_nr)
     for link in links
       let link_infos = vimwiki#base#resolve_link(link)
-      if !vimwiki#base#is_among_diary_files(link_infos.filename, copy(l:diary_file_paths))
+      if !vimwiki#base#is_diary_file(link_infos.filename, copy(l:diary_file_paths))
         let link_tpl = vimwiki#vars#get_syntaxlocal('Link1')
 
         let link_caption = vimwiki#base#read_caption(link_infos.filename)
@@ -1639,16 +1665,19 @@ function! vimwiki#base#follow_link(split, ...) abort
   " Try WikiLink
   let lnk = matchstr(vimwiki#base#matchstr_at_cursor(vimwiki#vars#get_syntaxlocal('rxWikiLink')),
         \ vimwiki#vars#get_syntaxlocal('rxWikiLinkMatchUrl'))
+
   " Try WikiIncl
   if lnk ==? ''
     let lnk = matchstr(vimwiki#base#matchstr_at_cursor(vimwiki#vars#get_global('rxWikiIncl')),
           \ vimwiki#vars#get_global('rxWikiInclMatchUrl'))
   endif
+
   " Try Weblink
   if lnk ==? ''
     let lnk = matchstr(vimwiki#base#matchstr_at_cursor(vimwiki#vars#get_syntaxlocal('rxWeblink')),
           \ vimwiki#vars#get_syntaxlocal('rxWeblinkMatchUrl'))
   endif
+
   " Try markdown image ![]()
   if vimwiki#vars#get_wikilocal('syntax') ==# 'markdown' && lnk ==# ''
     let lnk = matchstr(vimwiki#base#matchstr_at_cursor(vimwiki#vars#get_syntaxlocal('rxImage')),
@@ -2639,10 +2668,13 @@ function! s:clean_url(url) abort
 endfunction
 
 
-function! vimwiki#base#is_among_diary_files(filename, diary_file_paths) abort
-  " Check if filename is in a list of diary files
+function! vimwiki#base#is_diary_file(filename, ...) abort
+  " Check if 1.filename is a diary file
+  " An optional second argument allows you to pass in a list of diary files rather
+  " than generating a list on each call to the function.
+  let l:diary_file_paths = a:0 > 0 ? a:1 : vimwiki#diary#get_diary_files()
   let l:normalised_file_paths =
-        \ map(a:diary_file_paths, 'vimwiki#path#normalize(v:val)')
+        \ map(l:diary_file_paths, 'vimwiki#path#normalize(v:val)')
   " Escape single quote (Issue #886)
   let filename = substitute(a:filename, "'", "''", 'g')
   let l:matching_files =
@@ -2651,32 +2683,6 @@ function! vimwiki#base#is_among_diary_files(filename, diary_file_paths) abort
 endfunction
 
 
-function! vimwiki#base#is_diary_file(filename, ...) abort
-  " Check if filename is a diary file.
-  "
-  " For our purposes, a diary file is any readable file with the current wiki
-  " extension in diary_rel_path.
-  "
-  " An optional second argument allows you to pass in a list of diary files
-  " rather than generating a list on each call to the function.  This is
-  " handled by passing off to is_among_diary_files().  This behavior is
-  " retained just in case anyone has scripted against is_diary_file(), but
-  " shouldn't be used internally by VimWiki code.  Call is_among_diary_files()
-  " directly instead.
-
-  " Handle the case with diary file paths passed in:
-  if a:0 > 0
-    return vimwiki#base#is_among_diary_files(a:filename, a:1)
-  endif
-
-  let l:readable = filereadable(a:filename)
-  let l:diary_path = vimwiki#vars#get_wikilocal('path') .
-        \ vimwiki#vars#get_wikilocal('diary_rel_path')
-  let l:in_diary_path = (0 == stridx(a:filename, l:diary_path))
-  return l:readable && l:in_diary_path
-endfunction
-
-
 function! vimwiki#base#normalize_link_helper(str, rxUrl, rxDesc, template) abort
   " Treat link string towards normalization
   " [__LinkDescription__](__LinkUrl__.__FileExtension__)

doc/2023-03-23-file-link-resolution.wiki

@@ -0,0 +1,171 @@
+= Contents =
+  - [[#VimWiki file link resolution]]
+    - [[#context]]
+  - [[#possible solutions]]
+    - [[#revert to original behavior]]
+    - [[#check if a file exists on disk when following link]]
+    - [[#add a link scheme that means "open in vim"]]
+    - [[#add a configuration toggle for opening file: links in vim]]
+    - [[#add configuration for a set of filetypes to open in vim]]
+  - [[#open questions]]
+
+= VimWiki file link resolution =
+
+A decision document started by Brennen Bearnes on 2023-03-23.
+
+== context ==
+
+https://github.com/vimwiki/vimwiki/issues/950
+
+https://github.com/vimwiki/vimwiki/commit/d7ec12645a0460a7d200279c52915e6e080e9869
+
+Commit d7ec126 applies the wiki extension to files containing a `.`, thus
+allowing page names to contain a literal period.  This is correct behavior,
+insofar as pages like `Franklin D. Roosevelt` should be able to exist and get
+mapped to e.g. `Franklin D. Roosevelt.wiki`.
+
+Unfortunately, this broke behavior that a number of users rely on:
+
+https://github.com/vimwiki/vimwiki/issues/1271
+
+    I have some latex files stored and linked to in my wiki.
+
+    Before d7ec126 I could omit the file: part of a link to have vim open the file
+    directly.
+
+    After d7ec126 vimwiki adds a `md` extension when opening any file which
+    results in a `filename.tex.md` being created alongside the original tex
+    file. Using `file:` is not a solution since this can only be configured to
+    open a second instance of vim.
+
+It makes sense that this is useful.
+
+Sidebar: `file:` _can_ be configured to do just about anything with a custom
+handler, but that doesn't mean it's a very good user experience.
+
+= possible solutions =
+
+== revert to original behavior ==
+
+As a default, I personally think this is a non-starter.  Not being able to use
+`.` in page names is broken.  I also suspect other things break if you treat
+those files as wiki pages.
+
+We could make it configurable, though.
+
+*Pro:*
+
+  - People can go back to their expected behavior by setting one config
+    variable.
+
+*Con:*
+
+  - Adds an extra branch to link handling code.
+  - I haven't thought through the implications of treating it as fully
+    supported behavior.  What else will people expect to work right?
+
+== check if a file exists on disk when following link ==
+
+The idea is that if a link is `[[foo.txt]]` and `foo.txt` already exists, we
+should edit the existing file instead of creating a `foo.txt.md`.
+
+*Pro:*
+
+  - Might be the closest to expectations of people who rely on the old
+    behavior.
+  - Doesn't expand the language itself.
+
+*Con:*
+
+  - Complicates link handling code.
+  - Parsing a page requires knowing about external files.
+  - Might be surprising / confusing.  You can write `foo.txt` and wind up with
+    two different outcomes depending on whether `foo.txt` already exists.
+    - It's very Do What I Mean, which isn't necessarily bad.  But if I write a
+      link to `foo.txt` and wind up at `foo.txt.md`, it's not obvious that's
+      what I meant.
+
+== add a link scheme that means "open in vim" ==
+
+Something like `[[edit:foo.txt]]`.
+
+I vaguely think there's prior art for this in some older browsers?  I could
+swear Lynx had something like it, although I couldn't find anything under:
+
+  - [[https://en.wikipedia.org/wiki/List_of_URI_schemes|wp:List of URI schemes]]
+  - [[https://lynx.invisible-island.net/lynx_help/lynx_url_support.html|list of Lynx URL schemes]].
+
+I may just be thinking of `lynxexec:`.
+
+*Pro:*
+
+  - People want this anyway, and a `VimwikiLinkHandler()` for using `file:` or
+    some custom prefix to handle it is a common tweak to configurations which
+    we already document.
+  - It doesn't need a file existence check.
+
+*Con:*
+
+  - Expands the language.
+  - How is this converted to HTML?
+    - What will Markdown parsers do with it?
+  - Right now there are at least 3 high-level paths for opening a link:
+    - Open in VimWiki
+    - Hand off to custom link handler
+    - Open with `vimwiki#base#system_open_link()`
+    - ...this would introduce a 4th.
+  - Unless there is prior art, this would be something that _looks_ like a
+    URL/URI, but isn't recognized outside the wiki.  That seems fine for
+    custom prefixes, but is it ok to introduce here?
+
+== add a configuration toggle for opening file: links in vim ==
+
+Add a boolean that says "just open `file:` and `local:` links in the editor".
+
+*Pro:*
+
+  - Probably a relatively simple change.
+  - Might meet some users' needs.
+
+*Con:*
+
+  - Loses the utility in linking to binary filetypes which are best opened via
+    the system handler (images, PDFs, etc.).
+  - Users are explicitly already relying on `file:` links to be distinct from
+    `[[file.foo]]` links.
+
+== add configuration for a set of filetypes to open in vim ==
+
+A variation on the toggle for `file:` links would be to allow configuring a
+_list_ of file extensions which should be opened with Vim rather than passed on
+to the system handler.
+
+Something like:
+
+{{{vim
+  let wiki.file_link_edit_ext = ['txt', 'csv']
+}}}
+
+*Pro:*
+
+  - Doesn't expand the language.
+  - Doesn't have any implications for HTML rendering.
+  - A lot of `VimwikiLinkHandler()` boilerplate that is just used for making
+    links open in Vim could go away.
+    - I have this in my own config, so it's an appealing feature.
+
+*Con:*
+
+  - Does require people to rewrite existing link collections, but that's
+    true of other solutions as well.
+
+*Unanswered:*
+
+  - Would there be any sensible default for this besides an empty list?
+    My guess is not.
+
+= open questions =
+
+  - What is the actual current behavior with Markdown?
+    - Does it differ meaningfully from vimwiki syntax?
+    - If so, why?

doc/design_notes.md

@@ -4,6 +4,9 @@ This file is meant to document design decisions and algorithms inside Vimwiki
 which are too large for code comments, and not necessarily interesting to
 users. Please create a new section to document each behavior.
 
+## Link resolution
+
+See [VimWiki file link resolution](./2023-03-23-file-link-resolution.wiki).
 
 ## Formatting tables
 

doc/vimwiki.txt

@@ -4019,8 +4019,6 @@ master is retained as a legacy mirror of the dev branch.
 
 This is somewhat experimental, and will probably be refined over time.
 
-2023.04.04~
-
 New:~
     * Issue #1261: Feature: Support for <mark> in Markdown
       highlighting with yellow background and back foreground
@@ -4029,8 +4027,6 @@ New:~
       Also add the |blockquote_markers| variable
 
 Fixed:~
-    * Issue #1267: Improve performance on link creation by speeding up
-      is_diary_file()
     * Issue #1229: Bug: VimwikiGoto without argument creates empty page
     * Issue #1316: Permit tags in heading
       and improve |VimwikiSearchTags| and |VimwikiRebuildTags|

test/api_base_is_diary_file.vader

@@ -1,25 +0,0 @@
-# Test vimwiki#base#is_diary_file() for various inputs.
-
-Execute (Check known good diary file):
-  VimwikiIndex 1
-  let link_infos = vimwiki#base#resolve_link('diary:2020-07-22')
-  Assert vimwiki#base#is_diary_file(link_infos.filename)
-
-Execute (Check known good diary file using is_diary_file, with list of all diary files - legacy interface):
-  VimwikiIndex 1
-  let link_infos = vimwiki#base#resolve_link('diary:2020-07-22')
-  let diary_file_paths = vimwiki#diary#get_diary_files()
-  Assert vimwiki#base#is_diary_file(link_infos.filename, diary_file_paths)
-
-Execute (Check known good diary file use is_among_diary_files, with list of all diary files):
-  VimwikiIndex 1
-  let link_infos = vimwiki#base#resolve_link('diary:2020-07-22')
-  let diary_file_paths = vimwiki#diary#get_diary_files()
-  Assert vimwiki#base#is_among_diary_files(link_infos.filename, diary_file_paths)
-
-Execute (Check for nonexistent diary file):
-  VimwikiIndex 1
-  Assert !vimwiki#base#is_diary_file('not-a-diary-file')
-
-Execute (Clean):
-  call ReloadVimwiki()