Add support for XData entries.

Hugo-Heagren commented 1 year ago

@XData entries are 'containers' for data, which is not itself a bibliographic entry---intended to be reused by other entries through an inheritance mechanism. See the BibLaTeX manual section 3.13.6.

I liked the idea of using these, and I thought making the inheritance work in ebib might be an interesting challenge. This is an issue which has been briefly raised before, and there is also at least one other person who thinks xdata support might be cool.

(It is worth mentioning that the @XData entry type does not appear in bibtex-biblatex-entry-alist, so users will have to add it themselves to make ebib aware of it. This is possible, so isn't a problem for this PR, but it would be good in future to update that variable. I might make a patch soon.)

So far, this PR:

recognises @XData entries and xdata fields
displays inherited values properly, as previously with crossref

There's a lot of other stuff which is relevant to this and I haven't begun yet, but I wanted to push what I had so far for critique---to see if anyone else is interested and if what I have so far could be improved. This is a draft PR at the moment.

Other things to do:

ebib-follow-crossref currently assumes that the only crossreferencing field is "crossref", and that that only has one key in it. This PR means that may no longer be true. I think it would be good to re-write the command so that;
- When there is only one crossreferencing key (a single xdata key or a crossref field with a single key) then we jump straight to that one
- When there is more than one, present the user with a choice (probably a completing-read list of keys) and jump to whichever they choose.
Include proper display of granular xdata referencing in non-xdata fields. If an entry's author field is something like "{xdata=Jones1999:_some_paper-author-1}", BibLaTeX will read this the same as the first ("1") author ("author") of the entry with key "Jones1999_some_paper". This would be quite a lot of work and perhaps computationally quite expensive, but it could be cool.
XData and crossref inheritance can cascade -- a normal entry can inherit data from an XData entry which itself inherits that data from another XData entry. This isn't supported at all at the moment, but I think it would be a good thing to do. It's not only an xdata issue though, so perhaps this could be a separate, future PR.
An implementation detail of what I have so far. Currently ebib--display-xdata-field takes care of error checking in a similar way to the clause about ("crossref" "xref" "related") in ebib--get-field-highlighted, so it is not included in that clause. This is rather inconsistent and could conceivably be improved and collated. I did look at trying to include the xdata checking in the clause, but the display functionality for xdata splits the field up into separate keys and then operates on them individually (so if there is one good key, and one with an error, they are displayed on separate lines and only the erroneous one is given the warning face). The other fields aren't like this, so I think trying to collate everything into one clause might be quite clumsy.

I'm not sure how much of this could or should go into this PR. I would be happy to do all the work, but it might be good to separate it into manageable chunks.

Finally, relations between this and other PRs:

235 introduces specific editing functions for different fields. If we're going to add good xdata support, then it would be a good idea (and fairly simple, with existing infrastructure) to add one for the xdata field. Which PR this should go in depends on what is merged and when.
265 adds clickable buttons for crossrefs. This functionality should also be expanded to the keys in the xdata field, but again, where this code goes depends on what is merged first (because some of the code from that PR could probably be reused for the xdata functionality).

Would love others' thoughts on any of this.

joostkremers commented 1 year ago

This looks very good! Would be great if you could implement it fully!

FWIW, I would merge the completion PR and the clickable buttons PR first.

Hugo-Heagren commented 1 year ago

FWIW, I would merge the completion PR and the clickable buttons PR first.

Cool, I'll finish up with tthose first then.

What do you think of the "implementation detail" inconsistency? Ideally I'd like all the functionality to be coded in the same way, otherwise it could be quite confusing for future maintainers?

Hugo-Heagren commented 1 year ago

Just pushed some new commits.

I've rebased over master since the completion PR was merged.

I've added support for granular xdata references, with very detailed error reporting (see the docstring -- I'm quite proud of how information I've got into some coloured text and a help message).

I've mostly made cascading inheritance work. There's one major bug with this which I can't seem to fix. Say we have a file like this:

@Book{cascading,
    xdata = {xdata_location_and_publisher}
}

@XData{xdata_location_and_publisher,
    xdata = {xdata_publisher, xdata_location}
}

@XData{xdata_location,
    location = {London}
}

@XData{xdata_publisher,
    publisher = {McPublisher}
}

What should happen is that xdata_location_and_publisher displays "London" for the "location" field, and "McPublisher" for "publisher", inheriting them from different entries. Then cascading should display the same values for those fields, inheriting them from xdata_location_and_publisher. The bug is that, at the moment, cascading displays what it should (correct, inherited values for both fields), but xdata_location_and_publisher doesn't. Stranger still, if I remove one of the xdata references in the xdata field in xdata_locationp_and_publisher, then it does display what it should for the remaining one. Running something like (ebib-get-field-value "location" (ebib--get-key-at-point) ebib--cur-db t nil) with eval-expression shows that (even when the field contains both references) the right information is being retrieved -- that call returns "London". But it isn't actually displayed in the entry buffer. Any ideas?

EDIT: this is fixed now. I needed to add better inheritance to ebib-get-entry.

Otherwise what's left is:

alter ebib-follow-crossref
write a better editing command for xdata fields
make xdata ref keys clickable, like crossrefs and files

Hugo-Heagren commented 1 year ago

Some more stuff.

There's some structural stuff:

ability to filter entries in ebib-read-entry and related functions. This was because I wanted to write some commands in which the user selected one from a limited subset of the current database's entries. I also foresee that this might be useful for other commands which do this (citar-select-ref has a similar filter option)
ebib--get-xref-alist is a function to get a list of keys used in crossrefs for an entry. This functionality is abstracted because its useful in a few different places.

This enables the user-facing 'real' stuff:

when editing an xdata field, ebib--edit-ref-list-field will only present @XData type entries to the user.
ebib-follow-crossref now prompts the user for a key when there is more than crossref key in the current entry
ebib-follow-crossref will (by default) automatically jump to the entry crossreffed in the current field if invoked while point is on a crossref field (xref, xdata or crossref). In a similar spirit, if on an xdata field with multiple references, it will only list keys used in that field. This can be disabled per-invocation with a prefix arg, or globally with the new user option ebib-follow-current-field-crossref.

Hugo-Heagren commented 1 year ago

For myself as a reminder: just noticed that related fields displayed as if there's an error (i.e. in ebib-warning-face) when there is more than one key in them. This shouldn't happen, and that's probably probelm with this PR.

Hugo-Heagren commented 1 year ago

just noticed that related fields displayed as if there's an error

Fixed in the latest push.

that's probably probelm with this PR

As it turns out, it actually wasn't. A previous commit assumed that (like crossref and xref fields) related could only take one key. But the BibLaTeX manual Sec. 2.2.2, related says that they can take a list. I've displayed them like the xdata field: with each key on a separate line, and keys which do not exist in the current database highlighted in ebib-warning-face and with a suitable help-echo message.

Hugo-Heagren commented 1 year ago

I think this is ready now, apart from possibly adding clickability to the printed field representations, but I would only want to do that once #265 has been merged.

Very pleased with this one!

Hugo-Heagren commented 1 year ago

Actually, this probably isn't quite ready. I've just realised that the facilities for xdata inheritance are always available, whereas really they should only be available when the dialect is biblatex -- bibtex's inheritance systems are much less sophisticated. I'll have a go at making this happen soon.

joostkremers commented 1 year ago

Actually, this probably isn't quite ready. I've just realised that the facilities for xdata inheritance are always available, whereas really they should only be available when the dialect is biblatex -- bibtex's inheritance systems are much less sophisticated. I'll have a go at making this happen soon.

I'm not sure I find that such a big deal, though. I mean, it'd be fine if it is limited to biblatex files, but on the assumption that bibtex files won't usually have an xdata field, won't it be harmless if it isn't?

Hugo-Heagren commented 1 year ago

This had a few conflicts with master. Pretty sure I've fixed it all now and it should be ready to merge. I think you're right -- we can leave the bibtex/biblatex distinction, at least for now. Maybe I'll get bored over the winter and want to write something, but functionaly I think its fine.

Hugo-Heagren commented 1 year ago

Just wanted to say: I think this is pretty much ready to merge. I had wanted, just for fun, to write a completion table which could be used alongside other completion to allow for intelligent completion on parts of granular xdata entries, using boundaries. I've written the table, but integrating it into the rest of ebib is fiddly and after playing with it a bit I can't make it work. It's a lot of effort for what would basically be a vanity project on a a little-used feature -- I can come back to it later. If you fancy taking a look, I'll leave the code up here.

joostkremers commented 1 year ago

I've been trying out this code a bit and in general it seems to work well. If made a few small cosmetic changes, and fixed one issue where an extra line ended up in the commit that shouldn't be there.

Now, Github cheerfully tells me that I can "[a]dd more commits by pushing to the xdata-support branch on Hugo-Heagren/ebib", but I must admit I haven't found the git invocation to do so...

Another issue I've run into is that adding a new @Xdata entry isn't as good an experience as it should probably be. The first problem is the key: by default, Ebib creates keys automatically, but for an @Xdata entry, it may not have the right data to do so. Second, when you change an entry's type to @Xdata, the fields in ebib-extra-fields are shown, but from a user's perspective there is no reason why these fields are shown and not any others.

The deeper problem here is that @Xdata entries aren't really entries at all, I think. They are more like @String definitions, albeit at a different level. Which raises the question whether there should perhaps be a different way of editing them.

On the other hand, they have a structure that's very similar to actual entries, so it makes sense to reuse the editing infrastructure.

This raises another question: how should @Xdata entries be stored in Ebib? Right now, they are in the entries field in the database, but @String and @Preamble definitions have their own fields, and that may make sense for @Xdata entries as well. However, you've just spent a lot of effort in making the field inheritance work, which requires that they are stored under entries.

Anyway, I'd be interested to hear what you think about the UI, because you'll probably be the primary user of this functionality.

Hugo-Heagren commented 1 year ago

Now, Github cheerfully tells me that I can "[a]dd more commits by pushing to the xdata-support branch on Hugo-Heagren/ebib", but I must admit I haven't found the git invocation to do so...

Well, Hugo-Heagren/ebib is just a github repo like any other, so I would imagine that you just make a pull request against it like you would with any other: push your local branch to your own github repo (i.e. this one, but not on the master branch), then make a pull request on my repo, using the branch you've just pushed as a base (I haven't done this before, I'm just guessing -- maybe I'm wrong).

The first problem is the key: by default, Ebib creates keys automatically, but for an @Xdata entry, it may not have the right data to do so.

I know what you mean, although (personally) I'm actually not too fussed about this. I quite like having to create xdata keys manually. Xdata entries are a bit like variables within a biblatex database, and I've found it's often useful to give them meaningful names. which are quite different from what would be generated automatically. That's just me though -- other users (if there are any?) might still like a mechanism for automatic generation.

Second, when you change an entry's type to @Xdata, the fields in ebib-extra-fields are shown, but from a user's perspective there is no reason why these fields are shown and not any others.

Again, I don't really mind this. What is shown depends on the value of bibtex-biblatex-entry-alist, which is customisable (and presumably will be customised by any user who cares enough about this sort of thing to be using xdata entries in the first place). I have the fields you mention shown by default, but I could have others as well if I used them in xdata entries a lot.

The deeper problem here is that @Xdata entries aren't really entries at all, I think. They are more like @String definitions, albeit at a different level. Which raises the question whether there should perhaps be a different way of editing them.

On the other hand, they have a structure that's very similar to actual entries, so it makes sense to reuse the editing infrastructure.

This raises another question: how should @Xdata entries be stored in Ebib? Right now, they are in the entries field in the database, but @String and @Preamble definitions have their own fields, and that may make sense for @Xdata entries as well. However, you've just spent a lot of effort in making the field inheritance work, which requires that they are stored under entries.

Personally, I think they're treated as and stored by BibLaTeX as entrries, so should be treated the same by ebib. They have the same structure as entires, and behave in many of the same ways (including inheritance -- xdata entries can inherit from each other). I think this should go for ebib's internal representation, and user-facing ui (possibly with some QOL tweaks). I am also concious that to do anything else would require building a separate system, to deal with Xdata entries, which would then have to interact with the entries system. This seems like a lot of bother in comparison with just using the entries system we already have.

joostkremers commented 1 year ago

Well, Hugo-Heagren/ebib is just a github repo like any other, so I would imagine that you just make a pull request against it like you would with any other: push your local branch to your own github repo (i.e. this one, but not on the master branch), then make a pull request on my repo, using the branch you've just pushed as a base (I haven't done this before, I'm just guessing -- maybe I'm wrong).

Actually, I once stumbled upon the right invocation by trial and error. My understanding of git is not deep enough to come up with it myself, though, and I forgot what I did back then...

The first problem is the key: by default, Ebib creates keys automatically, but for an @Xdata entry, it may not have the right data to do so.

I know what you mean, although (personally) I'm actually not too fussed about this. I quite like having to create xdata keys manually.

Yeah, I think the user should create the key manually, just like a @String abbrev is. It's just that the process isn't very intuitive right now. You need to create the Xdata entry, leave the entry buffer and then change the key. It would be better for Ebib to ask for a key the moment the user indicates they want to create an Xdata entry.

This means we either add a function ebib--edit-type-field which checks for Xdata and asks the user for a key, or we add a new command ebib-add-xdata, which does basically the same as ebib-add-entry but specialises on Xdata.

The latter option would mean that the user shouldn't be able to change the type of an entry to Xdata, though, which would require extra work if Xdata is part of bibtex-biblatex-entry-alist.

So I guess the first option is the better one.

Again, I don't really mind this. What is shown depends on the value of bibtex-biblatex-entry-alist, which is customisable (and presumably will be customised by any user who cares enough about this sort of thing to be using xdata entries in the first place). I have the fields you mention shown by default, but I could have others as well if I used them in xdata entries a lot.

Personally, I'd find it "cleaner" if the fields in ebib-extra-fields wouldn't show up for Xdata entries, though perhaps that should be customisable.

Personally, I think they're treated as and stored by BibLaTeX as entrries, so should be treated the same by ebib.

Actually, I'm mostly concerned with the UI: it has to make sense to the user. I do agree it's better to store them as entries, though. It's less work not having to write a separate component, and it's also better in the sense that there's less code to maintain. But I do think a few UI tweaks would be in order.

Hugo-Heagren commented 1 year ago

Yeah, I think the user should create the key manually, just like a @String abbrev is. It's just that the process isn't very intuitive right now. You need to create the Xdata entry, leave the entry buffer and then change the key. It would be better for Ebib to ask for a key the moment the user indicates they want to create an Xdata entry.

I think you're right.

So I guess the first option is the better one.

I agree with this as well. I think that we should always prompt for a key when changing the type to xdata and the current key is a temporary one (this covers creating xdata entries). I think it should be customisable though whether we prompt when changing a pre-existing entry with a non-temporary key, or whether we just leave the key. I for one prefer not every to change my bibkeys if I can help it. This could be a boolean defcustom variable.

There are also other non-standard entry types (at least there is @set), see 2.1.1 in the manual. Maybe a similar behaviour (always prompt for a manually-entered key) would be good for them too?

Personally, I'd find it "cleaner" if the fields in ebib-extra-fields wouldn't show up for Xdata entries, though perhaps that should be customisable.

I personally agree. I only really want to see the fields I define in the custom variable to always be there (though I leave it empty, so that's none), as well those with values. Again, might this be useful for @Set as well?

But I do think a few UI tweaks would be in order.

Great, I think I we're in agreement. I'll try to get those sorted.

Hugo-Heagren commented 1 year ago

I've done the UI tweaks and done some tidying up. I couldn't see the commits you had pushed -- if they're somewhere publicly accesibly I can pull them and them to the branch myself if you like? Your name will still be on them as the committer.

I had to make some edits to other parts of the codebase to facilitate the UI stuff. See especially 43e3f45.

joostkremers commented 1 year ago

I've done the UI tweaks and done some tidying up. I couldn't see the commits you had pushed -- if they're somewhere publicly accesibly I can pull them and them to the branch myself if you like? Your name will still be on them as the committer.

I haven't pushed them anywhere. I'll merge or rebase them when I'm ready to merge your branch.

I had to make some edits to other parts of the codebase to facilitate the UI stuff. See especially 43e3f45.

It might take a couple of days before I can take a look, but I'll try to get to it as soon as I can.

Hugo-Heagren commented 1 year ago

Gentle bump.

joostkremers commented 1 year ago

Merged. Thanks for the contribution. :slightly_smiling_face:

Now all we need is a section for the manual. If you feel up to it, it'd be great if you could write something. Otherwise, I'll see if I can put something together.

Hugo-Heagren commented 1 year ago

Nice! Thanks for helping at your end!!

joostkremers / ebib

Add support for XData entries. #268

235 introduces specific editing functions for different fields. If we're going to add good xdata support, then it would be a good idea (and fairly simple, with existing infrastructure) to add one for the xdata field. Which PR this should go in depends on what is merged and when.

265 adds clickable buttons for crossrefs. This functionality should also be expanded to the keys in the xdata field, but again, where this code goes depends on what is merged first (because some of the code from that PR could probably be reused for the xdata functionality).