Skip to content

feat(biblio): add citar-org-roam #6728

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

feat(biblio): add citar-org-roam #6728

wants to merge 4 commits into from

Conversation

bdarcus
Copy link
Contributor

@bdarcus bdarcus commented Aug 26, 2022 •
edited
Loading

This PR has three commits:

  1. Per https://discourse.doomemacs.org/t/citar-org-roam-for-biblio-module, this adds tighter integration between citar and org-roam. I have elected not to add a specific flag, and install and configure it for org-roam users. I can't see any downside to that.
  2. A bump commit, to incorporate a performance fix in parsebib.
  3. Change the vertico binding for n b to citar-open-notes (since this is supposed to be for notes), and the description to "Bibliographic notes." This is more correct for vertico, though it's maybe a bit awkward for ivy and helm, which only have single entry points? Also, I wasn't sure how to do the commit message to indicate this is specific to vertico.

@bdarcus bdarcus marked this pull request as draft September 3, 2022 15:40
@bdarcus
Copy link
Contributor Author

bdarcus commented Sep 3, 2022

Ran into an issue; converting this to draft.

hlissner
hlissner requested changes Sep 6, 2022
View reviewed changes
@hlissner hlissner added is:feature Adds or requests new features, or extends existing ones module:lang/org Pertains to Doom's :lang org module module:tools/biblio Pertains to Doom's :tools biblio module re:org-roam Has to do with org-roam labels Sep 6, 2022
@bdarcus bdarcus force-pushed the feat-cor branch 2 times, most recently from b4be3a4 to 1f27e29 Compare September 13, 2022 11:24
@bdarcus bdarcus marked this pull request as ready for review September 16, 2022 23:05
@bdarcus bdarcus requested a review from hlissner September 16, 2022 23:25
@bdarcus bdarcus requested a review from a team as a code owner September 22, 2022 15:17
@bdarcus bdarcus force-pushed the feat-cor branch 2 times, most recently from c9fd3f2 to 8e73530 Compare September 27, 2022 18:37
hlissner
hlissner requested changes Oct 28, 2022
View reviewed changes
Copy link
Member

@hlissner hlissner left a comment

Choose a reason for hiding this comment

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

Sorry for the late response. Would you mind:

  1. Rebumping the bump commit,
  2. Resolving the README.org conflict,
  3. Reword the summary line of fix: bind "n b" to citar-open-notes to tweak(default): bind "n b" to citar-open-notes.

Then this should be good to merge.

@hlissner hlissner added this to the modules v22.10 milestone Oct 28, 2022
@bdarcus
Copy link
Contributor Author

bdarcus commented Oct 28, 2022 •
edited
Loading

Sorry for the late response. Would you mind:

  1. Rebumping the bump commit,
  2. Resolving the README.org conflict,
  3. Reword the summary line of fix: bind "n b" to citar-open-notes to tweak(default): bind "n b" to citar-open-notes.

Then this should be good to merge.

I always seem to run into some git issue or another on these PRs.

So I used the github conflict resolve UI for the README, which caused the following problem: it used a merge commit, and now I don't now how to edit the history to squash it with the citar-org-roam commit.

Can you possibly fix it, or give me any tips?

Otherwise, I've fixed the other issues.

@hlissner hlissner added the was:moved Is, was, or will be addressed elsewhere label Oct 28, 2022
@hlissner hlissner closed this in ee0e87f Oct 28, 2022
hlissner pushed a commit that referenced this pull request Oct 28, 2022
@bdarcus @hlissner
tweak(default): bind "n b" to citar-open-notes
9193ba9 
Also change desc to "Bibliographic notes".

Ref: #6728
hlissner pushed a commit that referenced this pull request Oct 28, 2022
@bdarcus @hlissner
bump: :tools biblio
714aeb0 
emacs-citar/citar@146f2cb5a31d -> emacs-citar/citar@4a302fcc405d
joostkremers/parsebib@175a1bdac1ea ->  joostkremers/parsebib@1efca921cbb4
andras-simonyi/citeproc-el@406bd9964f1c -> andras-simonyi/citeproc-el@36c4ecdc485a
tmalsburg/helm-bibtex@ce8c17690dda -> tmalsburg/helm-bibtex@78f5931e1cc8

Ref: #6728
@hlissner
Copy link
Member

It would probably have been easiest to rebase everything onto master before amending those commits, but I've cherry-picked the commits onto master directly to save you the trouble, so we can consider this merged. Thanks again for your help!

@bdarcus
Copy link
Contributor Author

bdarcus commented Oct 28, 2022 via email

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

@hlissner hlissner hlissner requested changes

Assignees
No one assigned
Labels
is:feature Adds or requests new features, or extends existing ones module:lang/org Pertains to Doom's :lang org module module:tools/biblio Pertains to Doom's :tools biblio module re:org-roam Has to do with org-roam was:moved Is, was, or will be addressed elsewhere
Projects
None yet
Milestone
modules v22.10
Development

Successfully merging this pull request may close these issues.
2 participants
@bdarcus @hlissner