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

Add support for Pandoc-style BibTeX citations #124

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Feb 6, 2021

Add features for pandoc-style citation of BibTeX references such as:

  • syntax highlighting
  • autocompletion based on a global .bib file configured in settings
  • peek / go-to definition
Completion Definition
citations-completion citations-peek-definition

@thomaskoppelaar
Copy link
Contributor

This is pretty cool, and I don't know if there are other extensions related to this type of functionality, but it might be worth looking for a different extension to merge this feature into.

@kortina
Copy link
Owner

kortina commented Feb 16, 2021

This is actually very cool.

A few comments.

@thomaskoppelaar how strongly do you feel this is better suited for another extension versus this one? It is a little tangential, but related, and I could see us merging it in.

@Anderas

  1. How do these references ultimately render in the markdown viewer? Does anything special happen / do the correct links get inserted?
  2. I think we should no-op all of the parsing / any expensive computation if there is no .bib file configured for the workspace, so this won't impact performance when parsing all of the notes in a large workspace for references.
  3. There are a few merge conflicts that need to be addressed.

@thomaskoppelaar
Copy link
Contributor

@kortina I'm not a great authority on this, since I don't use these types of notations myself.

I do understand the use of merging the functionality into this extension though. Markdown Notes has a pretty solid framework that can be extended easily enough.

On the other hand, seeing how standalone the new features are, it might just be best creating a new extension for those who are interested in using it. I'm definitely willing to help out @Anderas create this for example.

All in all: Both choices are very much valid.

@Anderas
Copy link
Contributor Author

Anderas commented Feb 18, 2021

Thanks both for your feedback

@kortina

  1. Markdown preview really depends on how a given previewer is configured and is deliberately separate from the editing. By default its just going to show the same text (eg @turing_computing_1950) but I am personally using Markdown Preview Enhanced which integrates with pandoc on the rendering front and is thus able to handle these references in a variety of configurable ways (pandoc is indeed very versatile). For example this is my own setup that uses Chicago-style references within text and automatically adds full-length references at the end.
Rendering
Screenshot 2021-02-18 at 19 14 34
  1. Good point, I added a check around the reference parsing and the rest should be fine already given early exit when trying to load the bibtex file
  2. Done

@thomaskoppelaar

I personally think that references sit quite nicely with the rest (tags, links, wiki) as they are about interlinking a single note with a variety of other sources. Also the code changes were indeed very easy to add to the this project given its relatively generic structure, not sure how much work it would be integrating it in another project. Do you have some specific one in mind? Or perhaps a brand new extension?

@@ -29,6 +29,18 @@
"name": "support.function.text.markdown.notes.tag.title"
}
}
},
{
"match": "(?<= |,|^)(\\@)([\\p{L}\\d\\-_]+)",
Copy link
Owner

Choose a reason for hiding this comment

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

Looking at this again -- I think this would match, for example, a string like @jordan23 -- am I correct in reading your description of the bibtex notation syntax that the string will always end in a 4 digit year?

(eg @author_title_year)

If so I am thinking we should update this regex (and the others) to have a _\\d{4} at the end of the word?

Copy link
Contributor Author

@Anderas Anderas Feb 19, 2021

Choose a reason for hiding this comment

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

Hm this could be a problem, because the citekey can have an arbitrary format (eg @author_title or even just @author) and different people use different styles (I just happen to prefer @author_title_year). This means the citekey is generally ambiguous between pandoc-style citation and github/twitter handles. Even the tests I added assume it can be of @string format. I see three ways to resolve the worry you raised:

  1. We limit citations to the stricter form, where they need to appear inside square brackets only, eg [@reference] instead of just @reference. This is the most commonly used style in pandoc as shown here. Pandoc does however support citations outside of brackets as well (and so does Markdown Preview Extended) so we would be restricting that functionality.
  2. Accept that @reference will be ambiguous between citations and handles with the following consequences:
    • syntax highlighting will always work even for github/twitter handles (maybe thats actually a good thing?)
    • if no global bibtex file is set, no parsing, autocompletion or go-to definition will work on handles
    • if global bibtex file is set and one markdown file contains both handles and citations, autocomplete will be suggesting references instead of usernames (but one can just ignore those suggestions) and go-to definition will only work on citations, whereas it will show the standard "No definition found" on handles
  3. Add a new setting that will let users specify what format of citekey they use, and we would then use that inside of the regex

What do you think about these three options?

Copy link
Owner

Choose a reason for hiding this comment

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

I like something along the lines of your (2).

I don't think we can conditionally do syntax highlighting, since this is governed by syntaxes/notes.tmLanguage.json not runtime code, but perhaps we could just document how the syntax highlighting works and have an FAQ like, "How can I disable syntax highlighting for @words / BibTex citations?"

https://github.com/kortina/dotfiles/blob/76780a8732d4af955a361b80022295e6650b85c3/vscode/settings.json#L347

Parts (b) and (c) of your (2) seem good. Can you remind me -- is that how the current implementation works?

If so, I think we can just merge this in and then async you could send over an FAQ commit either as part of this PR ( here ) or as a comment here if this merges in before writing it up and then I can add it to the FAQ.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes 2b and 2c are already working this way at the moment and I've now added a section into FAQ about overriding the highlight. Is this what you had in mind?

@kortina
Copy link
Owner

kortina commented Feb 19, 2021

@Anderas I think this is a pretty cool / natural thing (similar to #tags in how it's related) to add, so unless @thomaskoppelaar has a suggestion for another great home for it, I like the idea of merging in.

I left one more comment about the patterns. I can imagine lots of instances where someone is referencing an @username (eg, GitHub, Twitter) in a markdown file, so I think anything we can do to avoid over-matching on things like that will be appreciated by users who don't these mistakenly flagged. Of course, we could still have the case where someone references a username that ends in _\\d{4} but I think that is unlikely / acceptable casualty / not much we can do about it.

@thomaskoppelaar
Copy link
Contributor

Doing a very quick search gave me Bibmanager, but come think of it, its overall more efficient to merge it in this project.

@@ -74,7 +74,7 @@ export class NoteWorkspace {
static _rxBeginTag = '(?<= |,|^)#'; // match # preceded by a space, comma, or newline, regardless of whether it is followed by a letter character
static _rxWikiLink = '\\[\\[[^sep\\]]+(sep[^sep\\]]+)?\\]\\]'; // [[wiki-link-regex(|with potential pipe)?]] Note: "sep" will be replaced with pipedWikiLinksSeparator on compile
static _rxTitle = '(?<=^( {0,3}#[^\\S\\r\\n]+)).+';
static _rxMarkdownWordPattern = '([_\\p{L}#\\.\\/\\\\]+)'; // had to add [".", "/", "\"] to get relative path completion working and ["#"] to get tag completion working
static _rxMarkdownWordPattern = '([_\\p{L}\\d#\\.\\/\\\\]+)'; // had to add [".", "/", "\"] to get relative path completion working and ["#"] to get tag completion working
Copy link
Owner

Choose a reason for hiding this comment

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

Last thing before merge. This looks like a mistake, maybe -- a random digit has been added to the markdown word pattern?

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added on purpose: without it links and citations were highlighted (with cmd+hover) without the number part (eg only author_title_ instead of author_title_year). And it seems that other links / references could also contain numbers to benefit from this.

@kortina kortina merged commit b29cb0d into kortina:master Feb 24, 2021
@kortina
Copy link
Owner

kortina commented Feb 24, 2021

I just merged this branch, pulled master ( b29cb0d ), ran the extension from vscode, opened up test.md, type @t and tried to get completions but didn't get any.

I also tried adding a .vscode/settings.json inside the test workspace:

{
    "vscodeMarkdownNotes.bibtexFilePath": "library.bib",
}

Still not working:

image

Ah, I had to use an absolute path:

    "vscodeMarkdownNotes.bibtexFilePath": "/Users/kortina/src/vscode-markdown-notes/test/library.bib",

It might be nice to treat this as a workspace relative path if it's not an absolute path. Any chance you could do a quick PR to support that? (eg on nix, only treat paths that begin with / as absolute, else as paths in the workspace? not sure in windows how to distinguish this, but can't be too hard).

Lmk and I think I'll wait to push out the next release to the extension marketplace.

Sorry, I should have noticed this sooner.

@kortina
Copy link
Owner

kortina commented Feb 24, 2021

We should probably also add this file to the test/.vscode/settings.json directory / workspace:

{
    "vscodeMarkdownNotes.bibtexFilePath": "library.bib",
}

@Anderas
Copy link
Contributor Author

Anderas commented Feb 24, 2021

I just merged this branch, pulled master ( b29cb0d ), ran the extension from vscode, opened up test.md, type @t and tried to get completions but didn't get any.

I also tried adding a .vscode/settings.json inside the test workspace:

{
    "vscodeMarkdownNotes.bibtexFilePath": "library.bib",
}

Still not working:

image

Ah, I had to use an absolute path:

    "vscodeMarkdownNotes.bibtexFilePath": "/Users/kortina/src/vscode-markdown-notes/test/library.bib",

It might be nice to treat this as a workspace relative path if it's not an absolute path. Any chance you could do a quick PR to support that? (eg on nix, only treat paths that begin with / as absolute, else as paths in the workspace? not sure in windows how to distinguish this, but can't be too hard).

Lmk and I think I'll wait to push out the next release to the extension marketplace.

Sorry, I should have noticed this sooner.

Yea this makes sense, made a follow-up PR here. It does feel however that vscode should support this kind of behaviour out of the box, but currently does not.

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 this pull request may close these issues.

3 participants