#30 Directory bookmarks for MPDel browser

Merged
DamienCassou merged 2 commits from jao/mpdel:directory-bookmarks into master 8 months ago
jao commented 9 months ago
Collaborator

A couple of changes that have been in hibernation for a while:

  • Ability to use paths as MPDel browser top level entries, where they just work like
    direct bookmarks to the subdirectory given by the path.

  • A code clean-up that eliminates a messy implementation of how browser children know about their parents, which get a new one that is nearly trivial thanks to one of navigel's generics.

Thanks!

A couple of changes that have been in hibernation for a while: - Ability to use paths as MPDel browser top level entries, where they just work like direct bookmarks to the subdirectory given by the path. - A code clean-up that eliminates a messy implementation of how browser children know about their parents, which get a new one that is nearly trivial thanks to one of navigel's generics. Thanks!
jao added 2 commits 9 months ago
DamienCassou reviewed 8 months ago
DamienCassou left a comment

Great job, thank you!

(string= "Music directory" name))))
(cl-defmethod navigel-parent-to-open (entity &context (major-mode mpdel-browser-mode))
(cl-defmethod navigel-parent-to-open (_e &context (major-mode mpdel-browser-mode))
DamienCassou commented 8 months ago

is there a reason why you are using cl-defmethod instead of the simpler navigel-method?

is there a reason why you are using `cl-defmethod` instead of the simpler `navigel-method`?
jao commented 8 months ago

hmm, i want the context to be specifically mpdel-browser-mode and don't care really care about navigel-app to be set, which is what i understand navigel-method would give me for free: am i mistaken?

hmm, i want the context to be specifically mpdel-browser-mode and don't care really care about navigel-app to be set, which is what i understand navigel-method would give me for free: am i mistaken?
DamienCassou commented 8 months ago

exactly. You could write something like:

(navigel-method mpdel navigel-parent-to-open
                (_e &context (major-mode mpdel-browser-mode))

this would make the new code closer to the existing one but feel free to keep your code as it is.

exactly. You could write something like: ```emacs (navigel-method mpdel navigel-parent-to-open (_e &context (major-mode mpdel-browser-mode)) ``` this would make the new code closer to the existing one but feel free to keep your code as it is.
entity))
(list (or (navigel-parent navigel-entity) 'browser)))
(cl-defmethod navigel-parent-to-open
DamienCassou commented 8 months ago

can you please separate methods with a blank line?

can you please separate methods with a blank line?
jao commented 8 months ago

er, do you mean with an additional blank line?

er, do you mean with an *additional* blank line?
DamienCassou commented 8 months ago

No, it's fine. I guess I didn't see it :-).

No, it's fine. I guess I didn't see it :-).
DamienCassou merged commit 6aec060a80 into master 8 months ago
Poster
Owner

Thank you very much.

Thank you very much.
jao commented 8 months ago
Poster
Collaborator

thanks to you, damien, and happy new year!

thanks to you, damien, and happy new year!
jao deleted branch directory-bookmarks 8 months ago
The pull request has been merged as 6aec060a80.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.