Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maybe duplicated initialization #1810

Closed
zenyusy opened this issue Oct 13, 2024 · 1 comment · Fixed by #1811
Closed

maybe duplicated initialization #1810

zenyusy opened this issue Oct 13, 2024 · 1 comment · Fixed by #1811

Comments

@zenyusy
Copy link
Contributor

zenyusy commented Oct 13, 2024

lf/client.go

Lines 57 to 67 in 4de690d

if err := nav.sync(); err != nil {
app.ui.echoerrf("sync: %s", err)
}
if err := app.nav.readMarks(); err != nil {
app.ui.echoerrf("reading marks file: %s", err)
}
if err := app.nav.readTags(); err != nil {
app.ui.echoerrf("reading tags file: %s", err)
}

hi! lf is a very nice tool, thanks for creating and maintaining it.

seems like nav.sync already calls readMarks and readTags, is this redundancy necessary?

@joelim-work
Copy link
Collaborator

I had a look through the project history, it appears that nav.sync was originally supposed to only sync the list of cut/copied files, but this was extended to sync the list of marks as well. See #190 (comment):

I think maybe instead of adding a new command we can directly integrate marks syncing functionality to the sync command. I can't think of any downsides to this.

So I suspect that you are correct and the code on lines 61-67 aren't necessary. You are welcome to try it out yourself and submit a PR if that is the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants