-
Notifications
You must be signed in to change notification settings - Fork 2
Update fetch #51
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
Update fetch #51
Conversation
rlskoeser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a little bit of time end of day to test this out briefly, hope my comments are useful.
I looked up and found this command to determine current cache directory:
hugo config | grep cachedir
Maybe you've already found that; thought it could be helpful to look at the data you're getting back from the api call in the template. (I imagine eventually we'll want to recommend custom configuration or custom cache dir...)
layouts/shortcodes/bibliography.html
Outdated
| {{ $groupId := site.Params.groupId }} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a site configuration parameter, I suggest using a shortcode parameter - that will make the shortcode more flexible, since it could be used with multiple different ids on the same site. Maybe site config could be a fallback default, since it could be a common approach for sites like DHTech. (In which case, keep this for a later refinement once you get the basics working!)
https://gohugo.io/content-management/shortcodes/#arguments
I would go with named parameters. In future this could allow handling both group id and personal id in the same shortcode.
(I see now that you're using arguments for filters; I don't think that's necessarily incompatible with this....)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, thinking more about it, the cite tag would need to use the same zotero data as the biblography; so I guess it should either be a site parameter or a page parameter.
layouts/shortcodes/bibliography.html
Outdated
| {{- $authorParam := $context.Get "authorNum" | default "3" }} | ||
| {{- $authorNum := int $authorParam }} | ||
|
|
||
| {{ $endpoint := printf "https://api.zotero.org/groups/%d/items?start=0&limit=100&format=json" $groupId }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are functions for constructing urls (urls.JoinPath) and query strings (collections.Querify). Annoyingly it seems like you still have to use printf to combine them, but should make it more semantic to construct the api endpoint url and then set the parameters as a dictionary.
https://gohugo.io/functions/urls/joinpath/
https://gohugo.io/functions/collections/querify/#
Example (not connected, just constructing the two parts):
{{ urls.JoinPath "https://api.zotero.org/groups/" $groupId "items"}}
{{ collections.Querify (dict "start" 0 "limist" 100 "format" "json") }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic for getting the zotero data will need to be pulled out into a shared internal shortcode that both bibliography and cite shortcodes can use, since they need to work with the same data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Rebecca, I tried to pull out the fetching data but I think this would require the data be stored with scratch and then gotten from scratch in each of the shortcodes afterwards. I kept running into issues with scratch trying to do this even though I used scratch elsewhere in the shortcode. Do you know if there's a better way for this sort of application?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a partial template in another project that returns json data generated from templates, maybe it could be a useful example?
https://github.com/Princeton-CDH/lenape-timetree/blob/main/layouts/partials/leaf_data.json
| {{- end }} | ||
|
|
||
| <!-- adds entry to citations if it should be included based on filters and in-text citations --> | ||
| {{- if and $include (or (not $onlyCited) (in $citedTitles $entry.title)) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard for me to follow the filter logic above this, but when I disable the include check I get citations displaying. Without it, none of them display for me (even when I am not passing filters to the shortcode).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Rebeecca, I'm trying to make the filter logic more readable right now, but it was working correctly for me, what exactly led to none of the citations displaying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was testing on my personal site, where I started trying to integrate hugo-biblography - I wanted to do something like a customized version of the 'my publications' that zotero lets you set up on your personal profile.
However, in hindsight, that isn't the best place to test since I haven't gotten it fully working. What do you recommend for testing? Should I use the dh-tech site and switch to this branch for testing?
Some entries are still not being rendered despite all conditionals evaluating to true. More debugging is needed