Allow deleteTiddler to not modify fs #6047
Conversation
linonetwo
changed the title
| @@ -82,20 +82,20 @@ FileSystemAdaptor.prototype.saveTiddler = function(tiddler,callback,options) { | |||
| if(err) { | |||
| return callback(err); | |||
| } | |||
| $tw.utils.saveTiddlerToFile(tiddler,fileInfo,function(err,fileInfo) { | |||
| $tw.utils.saveTiddlerToFile(tiddler,fileInfo, (err,fileInfo) => { | |||
There was a problem hiding this comment.
I'm afraid we can't use arrow functions in the core. We stick to ES5.1 (roughly) because it's important to retain compatibility with older browsers/JS runtimes.
There was a problem hiding this comment.
I will keep that in mind in the following PRs.
| @@ -122,14 +122,22 @@ FileSystemAdaptor.prototype.loadTiddler = function(title,callback) { | |||
| }; | |||
|
|
|||
| /* | |||
| Delete a tiddler and invoke the callback with (err) | |||
| Delete a tiddler and invoke the callback with (err) | |||
| @params title title of tiddler to delete | |||
There was a problem hiding this comment.
We don't use jsdoc style comments
There was a problem hiding this comment.
Can there be a post in Github Discussion talking about this? Currently $tw API doc is incomplete, which makes there being some obstacles in plugin development.
| // Only delete the tiddler if we have writable information for the file | ||
| var cacheOnly = options.cacheOnly || false; | ||
| if(cacheOnly) { |
There was a problem hiding this comment.
The "cacheOnly" flag isn't used by the core, so I assume it is intended for plugins? It seems like it would be simpler if it was a separate method
There was a problem hiding this comment.
Ok, refactor complete!
There was a problem hiding this comment.
Feel free to point out if you don't like the function name. I think code review is a good place that let people know more about owner's style.
|
@Jermolene @linonetwo Looking over existing file-system issues. This caught my eye. I feel that this is actually a file-system-permissions issue. I did many tests with loading tiddlers from "read-only" files, and what happens when TW Node tries to delete them as one of my "fail gracefully" tests. At this point, there is no way for the TW Node server to set specific file permissions, so that would need to be done outside of TW code for the moment (or manually/on the command line). TW code like that leads into needing an "admin" authorization level to the server-flow, which I have started work on. Good issue! Definitely worth investigating user-facing solutions. |
|
@Jermolene Can you reconsider this? Just add a new method to allow plugin touch file cache. Just a refactor. No feature change in the core. |
@linonetwo I realised on review that we refer to boot.files as the "tiddler file info"
When writing watch-fs plugin that monitor fs change, I found it difficult to handle delete, file delete can happened by calling
deleteTiddler, or can be delete by the user using VSCode.So I need this new option to ask it to not touch fs.