Don't detect citations in code blocks

paleolimbot / rbbt

R Interface to the Better BiBTex Zotero Connector

142 stars 25 forks source link

Don't detect citations in code blocks #25

Closed AdrianHordyk closed 2 years ago

AdrianHordyk commented 2 years ago

I was having an issue where S4 slots in code blocks were being detected as citations.

Eg, rbbt::bbt_write_bib was resulting in error Slot not found with:

```{r eval=FALSE}

  MyObject[[1]]@Slot

``` #

My fix was to drop all code blocks from the content object. It works for me, but not sure if it is robust for all situations.

bwiernik commented 2 years ago

This could potentially be fixed, but detecting the context like that might be tricky. Have you considered using getSlot() instead of @?

AdrianHordyk commented 2 years ago

Yea, can use slot instead of @. Though it's not uncommon to access S4 objects in a list with list[[i]]@slot, and it wasn't obvious where the error was coming from until I did a line-by-line. So likely to be a problem for others again in this context.

Would it ever need to search for citations in a code block?

bwiernik commented 2 years ago

The problem is I'm not sure how easy or reliable it would be to detect if the citation is in a code block or not because that isn't really connected to the @ at all. @paleolimbot would be able to say more.

I know that using @ as an accessor is common, but I'd argue it's also a little against the S4 ethos to dig into the innards of an object manually rather than use methods.

paleolimbot commented 2 years ago

The flip side of this is that you might want citations that are included in a figure or table caption, both of which might be in a code block. This isn't the first time it's come up though and often S4 objects are poorly designed and using them reliably requires hacking with the @.

I'd be happy to merge this but I think a few changes are needed:

I think what you've proposed should be the default, but there should be an option (include_code_chunks) that can be set to TRUE to skip this step.
I think you can use stringr::str_remove_all() (slightly more compact)
There needs to be a test to verify that your code works! I believe there's a place for this already.

paleolimbot commented 2 years ago

I'm also pretty sure that you can have a backtick in a code chunk. Would .*? work instead of [^`]?

paleolimbot commented 2 years ago

This is awesome! I had this open on my computer so I pushed a version of this to master. Thank you!