Custom bibliography list class and attributes

[amoodie]

Summary

This PR adds support for custom bibliography list classes, and attributes. I.e, the changes, expose the bibliography_list_attributes config option as an option in the {% bibliography %} Liquid tag.

NOTE: I am not a ruby developer. In fact I have never edited ruby code before this PR. Please review carefully and critically.

Changes

1. Add an option to the utilities.rb optparse function named bibliography_list_attributes.
2. Add a function to utilities.rb named bibliography_list_attributes, which returns the passed bibliography_list_attributes argument if it was given, otherwise, returns an empty Hash {}.
3. The bibliography_list_attributes function is now called during the merge with config['bibliography_class'] during the call to render_item.
4. This merge enables custom HTML attributes to be passed, such as reversing the list with --bibliography_list_attributes {:reversed=>'reversed'}. Additionally, the class of the bibliography list can overridden by passing, e.g., --bibliography_list_attributes {:class=>'bibliography__custom'}.

Potential problems?

• uses a call to eval() on a string that the user supplies. My google searches reveal this may be a security issue. I can see how it would be to run on server-side code, but I'm not sure this is an issue for the plugin's users.
• the argument input to {% bibliography %} needs to be given as ruby Hash code. This may be confusing and problematic for people using Jekyll without digging into the code.
• I have never written cucumber tests before.
• I'm not sure the last cucumber test (Scenario: Reversed bibliography_list_attributes: {:reversed}) is actually written correctly. I.e., I'm not sure the test sets up the _config.yml file the way that a user would actually have it set up (below). I had to add a scholar_steps to process the bibliography_list_attributes to config.yml correctly. Maybe you can check this.

  scholar:
  bibliography_list_attributes:
  reversed: "reversed"

• reversed is a boolean html attribute to ol, so I don't think there is any way to "override" the reversed-ness of a list if bibliography_list_attributes is set in _config.yml as above.

Thoughts

• My use case is that I want to have a reversed (descending) bibliography on my "publications" page, but have other bibliography (blog posts, research project pages) use an ascending list. This code makes that possible, without breaking anything, but I don't know if it's the best way to do it.
• Do you think there is a better way to do this? Can we take a "list" similar to `--query`` and then process it into a ruby Hash behind the scenes? This may be easier for the user.
• Another option could be to just add an argument --reverse that inverts whatever the default bibliography_list_attributes is set in _config.yml.

[amoodie]

realized this probably addresses #234.

[inukshuk]

This is good work thanks!

I'm a little hesitant to merge this, because I'm a concerned that this is too much effort, or too complicated for something that should not be difficult. I don't mean this PR specifically, but rather that I was not anticipating the need for users to control the list generation in this way. I guess originally I was mostly thinking about styling issues and had the vague though that you could always just wrap your bibliography in a div with a custom class and then apply any custom styles you need in a flexible way.

Obviously, your use case here points to a flaw in that logic namely that the reverse attribute really is something that you can't add via a wrapper. Can you think of other examples where you absolutely need to control the list element itself? If there's just the one (and maybe class) we could turn those into separate options. This would at least remove the need for using eval; this is definitely something I'd like to avoid (thinking about scenarios where you have some setup similar to github-pages and multiple users can push their pages to it, this sounds like a dangerous thing to do).

[amoodie]

Hiya, yeah, I think you're correct that this is probably over complicated. It also feels wrong to expose the ruby Hash guts to the user.

I think that adding a --reverse option should be straightforward enough. This would be able to satisfy my use case, too. I'm currently building off my own fork, which is working for the moment, so when I find some time, I'll implement a --reverse option, and PR.

One question: what should the behavior of --reverse be when the default option for bibliography_list_attributes is set to be reversed already (e.g., the config below)? Should supplying --reverse in that case produce an 1) un-reversed list (i.e, an ascending list) or 2) should it do nothing?

scholar:
  bibliography_list_attributes:
    reversed: "reversed"

I guess we can close this PR and I'll send another when I get the chance?

[inukshuk]

Good question! I guess we could make it a boolean option bibliography_list_reverse that can be overridden in the bibliography tag using --reverse. That would probably require us to improve on the extremely simple attribute generation code here (i.e. skipping attributes if their value is false should be sufficient?).

[amoodie]

Okay, thanks for the response. I'll add this to the list to work on when I get a chance. I'll base changes from whatever the state of the repo is at the time, and ditch what I've done here, so I'm going to close this PR and open a new one in the future. Cheers.