Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(#1851): persist bookmarks#3033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
xiantang wants to merge8 commits intonvim-tree:master
base:master
Choose a base branch
Loading
fromxiantang:master

Conversation

@xiantang
Copy link
Collaborator

@xiantangxiantang commentedDec 22, 2024•
edited
Loading

Copy link
Member

@alex-courtisalex-courtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks great! I'll give it a full review and test in the next few days.

Initial thoughts:

  • Should we be usingvim.json.encode instead of the fn ?
  • Please don't move existing methods (constructor, toggle)
  • This should be an optional feature innvim-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

xiantang reacted with thumbs up emoji
@HadyMash

This comment has been minimized.

@alex-courtis
Copy link
Member

any updates?

I'm happy with this providing the above is addressed.

HadyMash reacted with thumbs up emoji

@xiantang
Copy link
CollaboratorAuthor

@alex-courtis updated

@alex-courtisalex-courtis changed the titleWIP: Support Persist Bookmarksfeat(#1851): persist bookmarksFeb 16, 2025
@alex-courtis
Copy link
Member

Proposed help, please correct indexes and runmake help-update when done.

============================================================================== 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.

Copy link
Member

@alex-courtisalex-courtis left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is looking fantastic! It's very robust and works well.

Please

end
file:write(vim.json.encode(data))
file:close()
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
end
end
else
notify.warn("Invalid marks.save_path, disabling persistence:"..errmsg)
opts.marks.enable_persistence=false

Printerrmsg 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

localstorepath=get_save_path(opts)
localfile=io.open(storepath,"w")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
localfile=io.open(storepath,"w")
localfile,errmsg=io.open(storepath,"w")

ifexplorerthen
for_,nodeinpairs(explorer.marks:list())do
status.bookmarks[node.absolute_path]=node.type
forkey,nodeinpairs(explorer.marks.marks)do
Copy link
Member

@alex-courtisalex-courtisFeb 16, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is private and should not be accessed, seeCI failure

Please useMarks:list()

I know this is not your code, but we can clean this up by usingself.explorer (guaranteed not null) instead of calling core.

Edit: it appears thatself.explorer.marks:list() is now returning an array of booleans instead of an array of nodes.

Comment on lines +514 to +517
marks= {
enable_persistence=false,
save_path=nil,-- nil will default to stdpath("data") .. "/nvim-tree-bookmarks.json"
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
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.

Seeproposed help

We then add the type explitictly so that it may be validated. Add toACCEPTED_TYPES:

bookmarks= {persist= {"boolean","string"},  },

@alex-courtis
Copy link
Member

I've added you to this repository so that you may push branches and run CI in the future.

@alex-courtis
Copy link
Member

Apologies, I was not clear about my request tosimplify options andproposed help

Attached is a patch with those changes

gunzip simplify_options.diff.gzgit apply simplify_options.diff

simplify_options.diff.gz

@bburgess19
Copy link

bburgess19 commentedMay 16, 2025•
edited
Loading

@xiantang I was looking for this feature! Could you kindly add the patched changes so we can all benefit from your implementation?

Thank you and@alex-courtis for working on this :)

@alex-courtis
Copy link
Member

@xiantang I was looking for this feature! Could you kindly add the patched changes so we can all benefit from your implementation?

Thank you and@alex-courtis for working on this :)

If we don't hear back from@xiantang I would be most grateful for a new PR from you@bburgess19 with all the changes.

@alex-courtis
Copy link
Member

@xiantang

Would you like me to complete this and merge it?

@xiantang
Copy link
CollaboratorAuthor

xiantang commentedJun 14, 2025 via email

ok i will continue it
…
On Sat, 14 Jun 2025 at 3:29 PM, Alexander Courtis ***@***.***> wrote: *alex-courtis* left a comment (nvim-tree/nvim-tree.lua#3033) <#3033 (comment)>@xiantang <https://github.com/xiantang> Would you like me to complete this and merge it? — Reply to this email directly, view it on GitHub <#3033 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIHB3TZ3Y5GOG3S2QK5HYUT3DPFNZAVCNFSM6AAAAABUBK6ZJGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNZSGQZDGOBVGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

@xiantang
Copy link
CollaboratorAuthor

xiantang commentedJun 14, 2025 via email

sorry for misunderstanding, u can complete this merge request.
…
On Sat, 14 Jun 2025 at 3:29 PM, Alexander Courtis ***@***.***> wrote: *alex-courtis* left a comment (nvim-tree/nvim-tree.lua#3033) <#3033 (comment)>@xiantang <https://github.com/xiantang> Would you like me to complete this and merge it? — Reply to this email directly, view it on GitHub <#3033 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIHB3TZ3Y5GOG3S2QK5HYUT3DPFNZAVCNFSM6AAAAABUBK6ZJGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNZSGQZDGOBVGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

@alex-courtis
Copy link
Member

sorry for misunderstanding, u can complete this merge request.

I would be very grateful if you completed. You deserve all the credit for this great feature.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@alex-courtisalex-courtisalex-courtis requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Persist Bookmarks

4 participants

@xiantang@HadyMash@alex-courtis@bburgess19
[8]ページ先頭
©2009-2025 Movatter.jp