align insert citation behavior, consolidate Embark maps

[roshanshariff]

• Context-aware insert-citation
  • Org mode (Make citar-insert-citation context aware #383)
  • Latex (already implemented)
  • Markdown
• Context-aware insert/edit function
  • Org mode (calls org-cite-insert)
  • Latex (calls citar-latex-insert-citation)
  • Markdown
• Target finders for keys and citations
  • Org mode
  • Latex
  • Markdown (based on Use thing-at-point for citar-markdown-key-at-point #395)
• Remove Org-specific Embark target (Custom completion categories and Embark keymaps #381)
• Update documentation

[roshanshariff]

I ended up not using org-cite-insert as the insertion function because its semantics don't really line up with Citar (i.e. change key at point, or edit citation style). Instead I wrote a new function that takes a list of keys (as required by the major mode functions) and adds those keys to the citation at point, skipping keys that are already included in the citation. The keys are added after the reference at point, or at the beginning/end of the citation if the point is in the prefix or suffix part.

[roshanshariff]

It looked pretty hard to modify the org-cite-insert logic into something that makes sense for Citar. In particular, it always insists on adding a single key if point is inside a citation (multiple keys are only allowed for new citations). And if the point is on a reference, it insists on changing that reference to a new one, rather than adding more references after it.

The function I wrote is still using all of the org-cite parsing machinery, but just adapts it to what we want for a Citar major mode function, which is to add keys to the current citation or insert a new citation.

[bdarcus]

it always insists on adding a single key if point is inside a citation (multiple keys are only allowed for new citations).

Yes, I agree that was a limitation.

And if the point is on a reference, it insists on changing that reference to a new one, rather than adding more references after it.

So how would you change one?

BTW, as you work on this, since you did roll your own, you might see how the shift reference (shift-left/right) and edit pre/post notes (M-p) functions work with it and affixes.

There were some issues with leading/trailing whitespace that I hadn't gotten around to.

And ideally the latter function would also edit global affixes (for the citation, rather than the reference) depending on point.

[roshanshariff]

I was thinking of splitting the keys-at-point major mode function into two:

• key-at-point returns a single key at point, along with its bounds. The bounds will include the affixes for the key if supported by the major mode.
• citation-at-point returns a list of keys, along with the bounds for the entire "citation object" if supported by the major mode.

So in Citar, we would have two target finders, one for keys and the other for citations. Since the functions return proper bounds, if you want to delete an entire citation/reference, you can just embark-act and then delete-region.

Then we add two new major mode actions to the "citation" target, backed by major mode functions:

• Edit citation: This shows a completing-read-multiple prompt with the existing keys already selected. If you deselect any keys, they get removed from the citation. If you select new keys, they get added to the citation after the reference at point. Maybe with a prefix arg it also prompts to edit the citation-level affixes?
• Change reference at point: Prompts for a single new key and replaces the one at point.

To back this up, we need a new major mode function, delete-references-from-citation. This plus the existing insert-citation should be all we need.

I'll take a look at the editing functions and see what they're doing now.

[bdarcus]

Maybe when you're "done" with the org one we can both test it, and the you can use what we conclude in the others.

[bdarcus]

I ended up not using org-cite-insert as the insertion function because its semantics don't really line up with Citar (i.e. change key at point, or edit citation style).

Can you clarify this last bit? Right now, I don't see how to edit the style. But it's pretty important.

[roshanshariff]

I just pushed a new version that seems to work reasonably well for org. You should be able to insert citations in org and latex, and in org-mode I added a new action called citar-edit-citation that calls org-cite-insert behind the scenes. This is bound to "E" in the citation keymap for now. So you need to embark-cycle until the entire citation is highlighted, then press "E". ("e" is also bound to "open bibtex entry" in citar-map, which is used for individual references, so you need to make sure you don't invoke that by mistake.)

Needless to say, there's work to be done in rationalizing the two remaining keymaps. There are no org-specific embark keymaps any more, just the two in citar.el. I've renamed citar-buffer-map to citar-citation-map. The two target types are called citar-reference (which was already there) and citar-citation (which takes over the duties of the oc-citation target type).

This should be a minimal working setup to test in org mode and at least reasonably okay in latex mode. I haven't really tested markdown yet.

[bdarcus]

So you need to embark-cycle until the entire citation is highlighted, then press "E".

I don't entirely understand this part. I don't have embark-cycle bound on my embark (doom) setup.

Also, I need to play with it more, but my initial impression is the edit citation at point UX is not ideal. I had thought you said above that you had figured out how to have the existing references pre-selected when bringing this up, which probably would be nice.

But without that, it seems more efficient and clear to just work with the references directly, using C-d, shift-left/right, and push/append of new references?

I think the more important citation-level editing would be of style/command, and (in biblatex and org) global affixes.

This should be a minimal working setup to test in org mode and at least reasonably okay in latex mode.

Editing an existing citation in latex doesn't really work correctly ATM, and the embark target finder doesn't appear to work.

Finally, org-cite-insert and citar-insert-citation don't behave consistently when I want to edit a style. Per above, we need style editing, and of course consistency.

[roshanshariff]

Yes, see my comment in #381 on consistency.

I hadn't yet implemented the edit-citation part (which would preselect the existing keys and allow you to delete them by deselecting). Currently it only adds new keys if they're not already present (so a context-sensitive insert citation). I do think the proper edit interface would be what I suggested, and will work on that once we've settled the keymaps/actions. It should be fairly easy to implement as a layer on top of the operations we already have.

The embark target finder does seem to work for me in Latex. Do you have an example of text where it doesn't work? It cycles between the key-at-point and the entire citation macro.

Sorry, I should've been clearer about embark-cycle. I just meant the operation that lets you choose between multiple potential targets (at least in the buffer; there's only one target when you're in the minibuffer). Pressing the embark-act key again when you're already in Embark invokes embark-cycle.

[bdarcus]

The embark target finder does seem to work for me in Latex. Do you have an example of text where it doesn't work? It cycles between the key-at-point and the entire citation macro.

It turns out a trivial problem: citep isn't included in the list of commands, so is not recognized. Maybe just add that while you're at it?

BTW, WDYT of my earlier suggestion we might want to distinguish natbib and biblatex? Can worry about it later, but just wondering.

[roshanshariff]

Yes, I do think we should try to be smarter about detecting which bibliography package somebody is using. I believe auctex has a whole system in place for class detection, so maybe we can take advantage of that. But I don't know much about it, so will look into it once the current batch of changes is done.

It would also be nice to have better editing support for biblatex multi-cite commands (which I remember you brought up somewhere recently). The new target finder does detect all keys in a multi-key command, but there's no editing support.

With the current code, you can edit the style in org mode by placing point on the style part, embark-act, and press E. If this doesn't work, it's a bug that I can fix.

[bdarcus]

Yes, I do think we should try to be smarter about detecting which bibliography package somebody is using. I believe auctex has a whole system in place for class detection, so maybe we can take advantage of that. But I don't know much about it, so will look into it once the current batch of changes is done.

A lightweight option is reftex-using-biblatex-p.

It would also be nice to have better editing support for biblatex multi-cite commands (which I remember you brought up somewhere recently).

AFAIK, it's not supported in bibtex-completion either, probably because it doesn't appear so straightforward to add.

Perhaps the approach @aikrahguzar took here makes it easier.

With the current code, you can edit the style in org mode by placing point on the style part, embark-act, and press E. If this doesn't work, it's a bug that I can fix.

That does work. But with point at same place, citar-insert-citation prompts for keys.

[roshanshariff]

it doesn't appear so straightforward to add.

True, but maybe a good project for a slow weekend 🙂

with point at same place, citar-insert-citation prompts for keys.

That was the behaviour I was going for, because I thought it made sense for insert-citation to only deal with adding new citations or inserting new keys into existing citations (and we're considering being able to delete keys with it, but now I realize that needs some thinking about). Whereas edit-at-point (aka org-cite-insert) is the operation to change the style or replace the key at point. Does this seem like reasonable semantics for those two operations?

[roshanshariff]

There's another reason for doing it this way: org-cite-insert prompts for different things based on where point is, so it's not really suitable as an embark action. If your current target is a citation key (from the minibuffer), but then you embark-act org-cite-insert with point on the style part, the citation key will get used as the new style. That's why it makes sense to ignore the current target for it (as you did in your code).

The insert-citation command always prompts for citation keys first, not styles or prefix/affixes.

[bdarcus]

I posted this before your latest post :-)

Will read that now.

Does this seem like reasonable semantics for those two operations?

I think the problem we have ATM is two different approaches:

1. one command for insert and edit, that is context aware (org-cite-insert)
2. one command for insert, and another for edit (what you're proposing)

I don't see how we can support both at the same time without confusing people. Do you?

Delete-keys, BTW, is supported now with C-d at point; but that's also context-aware: deleting the citation if point is on the citation, and deleting the key if on a reference.

https://github.com/bdarcus/citar/blob/0cee3c0d1180c7cb4938a8ddcca18a688b23d6c4/citar-org.el#L309-L312

I guess I don't see any downside in the org-cite-insert approach for a context-aware command, except that if you want to be able to edit the keys for the whole citation, you have to make a decision on which point to consider for that vs editing the style.

[bdarcus]

There's another reason for doing it this way: org-cite-insert prompts for different things based on where point is, so it's not really suitable as an embark action. If your current target is a citation key (from the minibuffer), but then you embark-act org-cite-insert with point on the style part, the citation key will get used as the new style. That's why it makes sense to ignore the current target for it (as you did in your code).

Ah, right. That gets to my point earlier about maybe the embark menu doesn't have to do everything.

It gets tricky sometimes to figure out how all this should fit together.

[roshanshariff]

Just to clarify, my proposal is

1. A context-sensitive command to insert, which inserts a new citation, or adds new keys to the current citation.
2. A context-sensitive command to edit the single thing at point, which could be the style part or a single key.

So both commands are context-sensitive, but the second one has a narrower focus.

I don't think all the commands need to be accessible through the embark menu, but there needs to be one command that is accessible through the menu and does the Right Thing most of the time (which would be (1) in my proposal). The other command (2) is also available through the menu, but this is strictly for convenience, because it ignores the current target and just cares about the location of point.

EDIT: The reason I'm proposing this is for consistency and predictability. If I invoke (1), then I know that I'm going to be asked for some keys and those keys will show up in the document, as close to point as makes sense. It will never prompt for a new style just because of where point happens to be, which I might not be expecting. But if I know I want to change the style or something about an existing citation, then I position point on what I want to change and invoke (2).

[bdarcus]

EDIT: The reason I'm proposing this is for consistency and predictability. If I invoke (1), then I know that I'm going to be asked for some keys and those keys will show up in the document, as close to point as makes sense. It will never prompt for a new style just because of where point happens to be, which I might not be expecting. But if I know I want to change the style or something about an existing citation, then I position point on what I want to change and invoke (2).

How do we document that clearly vis-a-vis org-cite-insert?

Do we tell people not to use it?

Or we tell them the choice is up to them?

[roshanshariff]

How do we document that clearly vis-a-vis org-cite-insert?

Command (2) is exactly org-cite-insert in org-mode, so that should be easy. The behaviour I described is what org-cite-insert does already. So if they like that then they can just use (2) all the time.

EDIT: And when the point is not on a citation, (1) and (2) do the same thing, so it doesn't matter which one you pick.

[roshanshariff]

I've pushed a new version, following what you suggested in #381 (two keymaps, one for minibuffer targets and one for at-point, and most of the insert functions are only in the minibuffer map). The at-point map does have the binding "i" for citar-insert-edit, which is just a wrapper around org-cite-insert in org-mode and a similar small wrapper in latex-mode. As you said, maybe this is a good intermediate step for now and later we can reconsider simplifying further.

No rush to look at it, and I'll probably test things on and off the next few days. We can then work out what needs to be done before merging (in particular, rebasing on top of #395; in this branch I haven't touched the Markdown code). I will also update docstrings, readme, etc. where required.

[bdarcus]

I think it generally seems good to me, though it may be we'll have to merge (when otherwise ready) and see what people think.

Questions/issues:

1. should insert/edit use CR (as now), or CRM? (this is a trivial change, so can always wait on feedback; unless it's constrained by Embark)
2. append/push vs replace, delete (more complex)

On 2, basic use cases; I want to:

1. replace a key
2. add a key before an existing key
3. add a key after an existing key

Let's just talk about org, and note that ideally should work the same in latex and markdown, at least ultimately (if not in this PR).

1. No support for this directly, so would delete the key (for example, with C-d), and add a new one
2. Put key at beginning of existing key, or right before it, and insert new key (though note there's no check for duplicate keys)
3. As with 2, supported, context-aware.

[roshanshariff]

should insert/edit use CR (as now), or CRM? (this is a trivial change, so can always wait on feedback; unless it's constrained by Embark)

It's actually constrained by org-cite-make-insert-processor, which calls citar-org-select-key; that now uses CR to replace the key at point and CRM when making a new citation. If we want different behaviour, we will have to switch away from using Org-cite's logic. That will give us more freedom to design the UX at the cost of a little more work (but not too much).

But let's just design the UX we would like for those three use cases and deal with the implementation as needed:

1. I think I like the replace key/edit style meaning of the current insert/edit (i.e. org-cite-insert). The trouble with delete-then-insert is that an Org citation won't parse unless it has at least one reference. So if you deleted the only remaining key, it would no longer be a citation and you wouldn't be able to embark-act on it to add a new key. We could work around this by making delete preserve the @ character if there's only one key left, then having insert recognize that, but that seems error-prone and fragile.
2. As a user, I think I would do this by moving point before the current key and then using (3), mirroring what you normally do with text.
3. I like the idea of offering insert-citation as the basic operation for adding new keys (i.e. binding it in the at-point keymap, not just for minibuffer candidates). They they will always go to the closest sensible place to point (after current key, after prefix, before suffix, or make new citation). We need to decide whether this uses CR or CRM.

I've never gotten Org-cite activation (and therefore the keymap at point) to work properly for me. I've been meaning to investigate why but haven't gotten around to it yet. But to make (2) easier maybe we can add some point-navigation commands to that keymap in addition to the shift-reference commands.

[bdarcus]

The trouble with delete-then-insert is that an Org citation won't parse unless it has at least one reference.

Right.

I've never gotten Org-cite activation (and therefore the keymap at point) to work properly for me.

Oddly, I find I run into times when it breaks, but very intermittently.

[roshanshariff]

though note there's no check for duplicate keys

Just to clarify, do you mean there is no check currently or there should be no check? If duplicate keys should be allowed, then maybe we can still use insert-citation but with de-duplication removed or made optional.

[bdarcus]

Just to clarify, do you mean there is no check currently or there should be no check?

The former.

[roshanshariff]

Hmm, so let's see what the behaviour should be: You put point on one of the references in a citation, invoke insert-edit, and select a reference that's already cited (or do we remove existing citations from the completion list?). Then one of the following things can happen:

1. Either the current reference or its duplicate gets deleted. But then this will lose the affixes for those references, which seems undesirable.
2. An error is signalled, and/or the user is asked to select a different reference. Personally, this feels too overbearing. We shouldn't try so hard to block something the user asked for, especially since they might legitimately want duplicate references? (With different locators or affixes, for example.)
3. The current reference is left unchanged, and point is moved to the duplicate to signal why. But then this fails to edit the current reference, which is what the user wanted.
4. The duplicate is allowed.

[roshanshariff]

I think you might have some code lingering around from the main branch, probably the Embark target finder? That screenshot show's it acting on a target of type "oc-citation" which isn't there any more in this branch.

[bdarcus]

Yeah, was some git/straight weirdness I think. Fixed now.

Is the highlighting of the targeted here something your code does?

[roshanshariff]

Yes, the new Org and Latex target finders I wrote return not just the key/citation at point, but also the bounds. Embark then shows these bounds as highlighting.

[bdarcus]

The styling there is problematic. So is that likely a doom theme issue?

EDIT: yes, embark-target, which inherits from highlight.

[bdarcus]

I haven't really tested markdown yet.

I just tested a bit, but ran into a few issues (which shouldn't be specific to markdown, but ...).

1. citar-key-finder needs to be autoloaded
2. citar-keys-at-point no longer present; changing that call to the above seems to fix that
3. I get the "citation insertion unsupported for markdown-mode" error, which is odd; I sometimes get this in org too, BTW, so might be some load-order thing?

Also, minor thing: should we change keys-at-point -> key-at-point?

[bdarcus]

Suggested change

	(defun citar-key-finder ()
	;;;###autoload
	(defun citar-key-finder ()

[roshanshariff]

Right, I'll have some time this weekend to finish the Markdown integration and get this merged. That'll involve doing a rebase onto the main branch (and probably opening a new PR against the main branch, instead of this one).

There are now two major-mode functions, insert-citation and insert-edit. Org has a context-sensitive implementation of the former and calls org-cite-insert for the latter. But currently Markdown only has the former (and not context-sensitive), so you'll get an error if you run citar-insert-edit.

There shouldn't be any load-order dependent errors. But if you're switching branches and re-evaluating buffers, there's some possibility that the citar-major-mode-functions variable doesn't get updated. You might have to eval-defun it to make sure it gets its new value. The "not supported" errors come from missing entries in citar-major-mode-functions, regardless of whether a function is loaded or not.