Allow WikiPageRename to move the page to another directory (including creating the directory)

[wasowski]

These are two tests that demonstrate how WikiPageRename works for moving pages around. It succeeds if target dir exists and fails if target dir does not exist. These tests are supporting discussion in issue #107

[wasowski]

I also added a proposal how to check whether a target directory exists and produce a controlled error message. If this is what you want then we should revert the second test as success now, and start working on WikiPageRename! (or WikiPageMove?). See issue #107.

[lervag]

From #107:

wiki#page#rename fails with a generic message Cannot rename if the target directory does not exist. This makes it difficult (or more difficult than need be) to move pages around. At the very least, I think the error message should be more specific, but I am wondering whether the directories should not be created automatically with something like mkdir -p before attempting to move. One possibility is to keep the current behavior of WikiPageRename as is (with a clearer error message) and add WikiePageRename! that would also create folders.

I fully agree that we should improve this.

I also added a proposal how to check whether a target directory exists and produce a controlled error message. If this is what you want then we should revert the second test as success now, and start working on WikiPageRename! (or WikiPageMove?).

I think a solution could be something like this:

• If the target directory does not exist, then ask the user if it should be created.

• If user responds no: abort.

• If user responds yes, then attempt to create the directory. If it fails, provide an error message.

Essentially, I think asking the user should be an OK way to handle it that should make the ! version redundant. But I'm open to objections/different opinions, of course!

[lervag]

Since you are already working on this in your fork I'll not work on it myself yet. But I'll be glad both to help you with the PR and also, if you prefer, to finalize the implementation.

[wasowski]

Thanks, I'll finish this. Just not sure when. It may take 2-3 days.

[lervag]

Great. And, as you also said in #107: no rush. :)

[wasowski]

Sorry for being quiet here. I am completely swamped, but I have not forgotten this. It just needs to wait a bit!

[lervag]

I know how it can be, no worries :)

[wasowski]

I wouldn't say this is done @lervag but I think you should have a look at this as I am ending in difficulties that show my lack of understanding of wiki.vim invariants. I think I (mostly) did what you suggested. I ask for the permission to create a new dir (in the 'ask' function) and then the non-interactive function should just succeed.

Still when testing I am arriving at many small problems that I am not sure if they are directly related to the patch, or whether they are problems overall that need to be looked at separately:

• The final part (update and bwipeout) has unintended side effects for me. It closes the buffers, and if the buffer is last in a tab then it also inadvertently looses the tab (with its name and context). I am rather uncertain if this was not a problem before already. In general I would prefer not closing the wiki buffers for updating links. Can't we update them somehow without wipeout? reload? why edit! is not good enough?
• The reopenning code does not really work for me. Renaming/moving seems to happen correctly in the file system, but the new buffer is not opened. I was unable to figure out why? Hints?
• I noticed that renaming links fails (ie. does not rename), if simplification has changed something. Perhaps renaming links should simplify links before comparing (or why is it a bad idea?). I did not have time to look into how link renaming works though. I think I will look in there after I get some comments from you.

In general I struggle with the 'cwd-invariant' - vim has a working directory active in the buffer we are renaming. This could be anything (in my tests it was often outside the wiki tree!). Typically vim file operations work with respect to the CWD, but here I would prefer to give path with respect to the position of the wiki file open, or with respect to the wiki root. Only the former works now. Perhaps I should implement a detection if someone starts with a slash? (or is there any machinery in place?)

As you see these are many questions. I think it would be best to address those that you consider bugs in this specific functionality (creating a dir) and separate the others into their own issues, to make it easier to work with in small chunks. Comments?

[lervag]

I wouldn't say this is done @lervag but I think you should have a look at this as I am ending in difficulties that show my lack of understanding of wiki.vim invariants. I think I (mostly) did what you suggested. I ask for the permission to create a new dir (in the 'ask' function) and then the non-interactive function should just succeed.

I think this actually looks good. I'm looking into it now if it might be merged already.

• The final part (update and bwipeout) has unintended side effects for me. It closes the buffers, and if the buffer is last in a tab then it also inadvertently looses the tab (with its name and context). I am rather uncertain if this was not a problem before already. In general I would prefer not closing the wiki buffers for updating links. Can't we update them somehow without wipeout? reload? why edit! is not good enough?

• The reopenning code does not really work for me. Renaming/moving seems to happen correctly in the file system, but the new buffer is not opened. I was unable to figure out why? Hints?

These looks unrelated to your patch, so I would be happy to look into them as one or more separate issues. Perhaps you could raise both of them? It would be useful with a simple example of how to observce each of them. You don't need many words for this, really, just the simple description and an example.

• I noticed that renaming links fails (ie. does not rename), if simplification has changed something. Perhaps renaming links should simplify links before comparing (or why is it a bad idea?). I did not have time to look into how link renaming works though. I think I will look in there after I get some comments from you.

This is not caused by simplify(), no. Again, I believe this is unrelated, and feel free to post an issue with an example.

In general I struggle with the 'cwd-invariant' - vim has a working directory active in the buffer we are renaming. This could be anything (in my tests it was often outside the wiki tree!). Typically vim file operations work with respect to the CWD, but here I would prefer to give path with respect to the position of the wiki file open, or with respect to the wiki root. Only the former works now. Perhaps I should implement a detection if someone starts with a slash? (or is there any machinery in place?)

My "philosophy" is to always work with absolute paths, because I can never assume anything about how a user "operates". I personally like to avoid keeping a distinct CWD for Vim. That is, I prefer to let CWD always be the same as the current file (set autochdir). I believe this might be uncommon. But both alternatives should work, and it typically does work as long as we ensure to use absolute paths in the code.

[lervag]

The code to check if the new directory exists is only applied for rename_ask: I'm curious if it would be better to move this code to rename() and add an extra option: e.g. ask_for_mkdir. If true, we ask before mkdir, else we run mkdir without asking. Or something like that.

Else I think this looks good now.

[lervag]

No need for this change?

[lervag]

You should use expand('%:p:h') to ensure you are using an absolute path. I would also move the simplify from line 131 to 130. I would also consider to combine lines 130 and 131, since l:newpath is not used any further. E.g.:

let l:target_dir = fnamemodify(
      \ simplify(printf('%s/%s', expand('%:h'), l:name)), ':h')

Finally, note that the expand() on line 131 is not necessary.

[lervag]

Please use two spaces per indent :)

[wasowski]

The code to check if the new directory exists is only applied for rename_ask: I'm curious if it would be better to move this code to rename() and add an extra option: e.g. ask_for_mkdir. If true, we ask before mkdir, else we run mkdir without asking. Or something like that.

hmm. I can of course do this. No problem. However, I thought that it was quite neat to separate a function that is interactive from the function that is non-interactive. Perhaps I have misread the original design intention (between wiki#page#rename and wiki#page#rename#ask). Let me know, and i can of course shift the code as stated above, if you still think so.

Also when renaming the page with the current patch, the path for the new file is interpreted from the position of the renamed page (relative to the renamed page). I think this is fine, but just wanted to make it clear, since you stated your 'absolute path' policy.

I have addressed the other minor issues, and will look into opening separate entries for the buffer problems now (done, issue #124).

[lervag]

The code to check if the new directory exists is only applied for rename_ask: I'm curious if it would be better to move this code to rename() and add an extra option: e.g. ask_for_mkdir. If true, we ask before mkdir, else we run mkdir without asking. Or something like that.

hmm. I can of course do this. No problem. However, I thought that it was quite neat to separate a function that is interactive from the function that is non-interactive. Perhaps I have misread the original design intention (between wiki#page#rename and wiki#page#rename#ask). Let me know, and i can of course shift the code as stated above, if you still think so.

Yes, I can see what you mean. However, I'm thinking the original function should also handle the missing directories. So, as a minimum, it should raise an error or something if the target directory does not exist. My suggested solution was to move the interactive handling of a missing directory to the main function and to add an argument to avoid the interactivity. I'm thinking: If we want to change a filename and possibly move it to a different directory, then the function needs to account for this problem. If we don't explicitly say how to solve it, then we should ask. Perhaps the argument should be mandatory and be one of [ask, abort, create], and default to abort?

Also when renaming the page with the current patch, the path for the new file is interpreted from the position of the renamed page (relative to the renamed page). I think this is fine, but just wanted to make it clear, since you stated your 'absolute path' policy.

Yes, good point. I think this is fine. My point about absolute paths was mainly that I try and always work with paths as absolute paths in the code. Not necessarily from the user perspective.

I have addressed the other minor issues, and will look into opening separate entries for the buffer problems now (done, issue #124).

Great! Let's merge this before working with #124, ok?

[wasowski]

Moved the directory creation code to wiki#page#rename with an optional dir_mode argument as you suggested. I hope I did not mess up with re-basing and force-pushing.

[lervag]

Thanks! I've merged and made some minor adjustments.