feat(#1851): persist bookmarks #3033
Conversation
There was a problem hiding this comment.
This looks great! I'll give it a full review and test in the next few days.
Initial thoughts:
- Should we be using
vim.json.encodeinstead of the fn ? - Please don't move existing methods (constructor, toggle)
- This should be an optional feature in
nvim-tree-opts, disabled by default - This needs checking and safety
- Use a pcall or similar to prevent an error being raised
- Report failures to the user
This comment has been minimized.
This comment has been minimized.
I'm happy with this providing the above is addressed. |
|
@alex-courtis updated |
alex-courtis
changed the title
|
Proposed help, please correct indexes and run ==============================================================================
5.19 OPTS: BOOKMARKS *nvim-tree-opts-bookmarks*
*nvim-tree.bookmarks.persist*
Persist bookmarks to a json file containing a list of absolute paths.
Type: `boolean` | `string`, Default: `false`
`true`: use default: `stdpath("data") .. "/nvim-tree-bookmarks.json"`
`string`: absolute path of your choice. |
| end | ||
| file:write(vim.json.encode(data)) | ||
| file:close() | ||
| end |
There was a problem hiding this comment.
| end | |
| end | |
| else | |
| notify.warn("Invalid marks.save_path, disabling persistence: " .. errmsg) | |
| opts.marks.enable_persistence = false |
Print errmsg and disable persistence on a bad path, permission etc.
[NvimTree] Invalid marks.save_path, disabling persistence: /home/alex/marks.json: Permission denied
[NvimTree] Invalid marks.save_path, disabling persistence: /home/alexxxxxxxxxxxxxxxxxxxx/mar ks.json: No such file or directory
| end | ||
|
|
||
| local storepath = get_save_path(opts) | ||
| local file = io.open(storepath, "w") |
There was a problem hiding this comment.
| local file = io.open(storepath, "w") | |
| local file, errmsg = io.open(storepath, "w") |
| @@ -208,8 +208,8 @@ function Filters:prepare(project) | |||
|
|
|||
| local explorer = require("nvim-tree.core").get_explorer() | |||
| if explorer then | |||
| for _, node in pairs(explorer.marks:list()) do | |||
| status.bookmarks[node.absolute_path] = node.type | |||
| for key, node in pairs(explorer.marks.marks) do | |||
There was a problem hiding this comment.
This is private and should not be accessed, see CI failure
Please use Marks:list()
I know this is not your code, but we can clean this up by using self.explorer (guaranteed not null) instead of calling core.
Edit: it appears that self.explorer.marks:list() is now returning an array of booleans instead of an array of nodes.
| marks = { | ||
| enable_persistence = false, | ||
| save_path = nil, -- nil will default to stdpath("data") .. "/nvim-tree-bookmarks.json" | ||
| }, |
There was a problem hiding this comment.
| marks = { | |
| enable_persistence = false, | |
| save_path = nil, -- nil will default to stdpath("data") .. "/nvim-tree-bookmarks.json" | |
| }, | |
| bookmarks = { | |
| persist = false, | |
| }, |
Rather than a toggle and path, we can use just one option that is boolean or string. This is consistent with other options.
See proposed help
We then add the type explitictly so that it may be validated. Add to ACCEPTED_TYPES:
bookmarks = {
persist = { "boolean", "string" },
},
|
I've added you to this repository so that you may push branches and run CI in the future. |
|
Apologies, I was not clear about my request to simplify options and proposed help Attached is a patch with those changes gunzip simplify_options.diff.gz
git apply simplify_options.diff |
close #1851 (comment)