Create new page with fzf even if there is a match#143
Create new page with fzf even if there is a match#143eldridgejm wants to merge 3 commits intolervag:masterfrom
Conversation
Previously, fzf#pages was only capable of creating a new wiki page if the query resulted in no matches. This commit allows for the creation of a new page even when the query does match one or more existing pages by pressing alt-enter while in the fzf prompt.
lervag
left a comment
There was a problem hiding this comment.
Thanks, I think these are good updates. Regarding keybinding: I assume it can not be "anything", so if it is customizable it will probably still be a choice? Also, the keybinding syntax is fzf specific, which differs from Vim notation. I'm curious, though, if alt+enter is the best choice: how about ctrl+enter? In my experience, people tend to use alt+... for global keymappings, so I would expect e.g. ctrl+enter to be less intrusive in this sense. But I'm not going to insist. Perhaps customizibility with a proper default is good here?
autoload/wiki/fzf.vim
Outdated
| call map(l:pages, '"/" . substitute(v:val, l:root . "/" , "", "")') | ||
| call map(l:pages, {_, x -> x . "¤" . fnamemodify(x, ':r')}) | ||
|
|
||
| let fzf_opts = join([ |
There was a problem hiding this comment.
I prefer to keep the empty line above to separate blocks of code. Also, although it is clearly opinionated, I've settled on the style of prefixing the l: to indicate local variables.
There was a problem hiding this comment.
Sure thing, I'll include these style changes in the next revision.
| " lines -- otherwise, three. The first line is the query, the second is | ||
| " either empty or alt-enter, depending on if enter or alt-enter was used to | ||
| " select, and the third line (possibly) contains the selection | ||
| if len(a:lines) == 2 || !empty(a:lines[1]) |
There was a problem hiding this comment.
Thanks, I agree it is good to have a comment here to explain.
autoload/wiki/fzf.vim
Outdated
| let l:page = a:lines[0] | ||
| redraw! | ||
| call wiki#log#info('Opening new page "' . l:page . '"') | ||
| call wiki#log#info('Opening page "' . l:page . '"') |
There was a problem hiding this comment.
Why did you remove the new here? The point was/is to indicate that it is indeed a new page, not an old, right?
Also: What happens now if we press alt-enter and the page already exists?
There was a problem hiding this comment.
If we press alt-enter and the page already exists, the existing page will be opened and "Opening page" will be displayed. This is also the reason for removing the new , as we are not necessarily creating a new page.
There is a decision to make as to what happens when alt-enter is pressed and g:wiki_map_create_page is defined. I see two natural possibilities:
- A page whose name is the query, verbatim, is opened/created as necessary.
g:wiki_map_create_pageis first applied to the query, and the resulting page name is opened/created as necessary.
The pull request currently implements the second approach. Personally, I feel it is the less surprising of the two, and the first approach would require complicating the logic.
As for the log message, I could add a check to see if the page already exists, and, if not, print that a new page is being created. One consideration here is that we'd need to calculate the effective name of the page by possibly applying g:wiki_map_create_page. This would involve duplicating a few lines of wiki#page#open. Maybe it makes sense to move the check for whether the page exists and the resulting wiki#log#info('Opening new page') to wiki#page#open itself? At first glance, that seems like a natural place for it.
There was a problem hiding this comment.
The pull request currently implements the second approach. Personally, I feel it is the less surprising of the two, and the first approach would require complicating the logic.
Yes, I guess you can say that; implicitly. The g:wiki_map_create_page is used behind the scene by wiki#page#open. I agree this seems to be the least surprising behaviour.
As for the log message, ...
I agree; moving the echoing inside the wiki#page#open makes sense. I think it should be possible to apply the logic to the parsed url; let l:url = wiki#url#parse() then check the resulting path before open().
|
Sure thing. In regards to the keybinding, I agree that making it an option and providing a good default seems to be the way to go. fzf does have a finite but expansive list of possible key combinations. Unfortunately, |
Thanks for the link and info; then it remains to decide the default.
Ah, yes; |
|
Thanks for the feedback! I have implemented all of the changes discussed. I've added a global configuration option, |
|
Merged now. I made a minor change to the accept_page comment, but other than that this is top quality PR. Thanks! |
Previously,
fzf#pageswas only capable of creating a new wiki page if the query results in no matches. This commit allows for the creation of a new page even when the query does match one or more existing pages by pressing alt-enter while in the fzf prompt.My typical use case for
fzf#pagesis to search for a page and open it if it exists -- otherwise, create it. I often find myself in a situation where the page I'm looking for doesn't exist, but my query does match some existing unrelated page, and so I cannot create the new page withfzf#pages. For example, I'm trying to create a page namedtest, but there's a page namedtechnical support(which matches due to having t,e,s,t, in that order).This pull request uses fzf's
--expectto allow the user to "force create" a page with the provided query as the name by pressing alt-enter instead of enter. I have left the original behavior, but in principle it could be removed to provide for a more consistent user experience. I have hardcoded the keybinding to avoid introducing more configuration, but it can be made customizable.