Skip to content
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

Center on new entry in ebib-biblio-import-doi #241

Closed
wants to merge 37 commits into from

Conversation

gkowzan
Copy link

@gkowzan gkowzan commented Jan 21, 2022

Fixes issue #232.

Return the list of entry keys of the added entries instead of just the number of
added entries as the first element of the returned list. `ebib-import-entries`
now also returns the list of added entry keys. Modify `ebib--bib-read-entries`
to use the new return value.
This version uses `biblio-doi-forward-bibtex` and a callback function to be
compatible with asynchronous url retrieval.
@gkowzan gkowzan force-pushed the center-on-new-entry branch from 4d2174d to dae41ca Compare January 21, 2022 16:39
joostkremers and others added 21 commits January 21, 2022 23:36
Given Github issues joostkremers#242 and joostkremers#245, it seems this functionality isn't ready to be
enabled by default yet.
Strings just are arrays, so this doesn't do anything useful, and adds
extra dependency for users.
ebib--expand-string: remove redundant `dbus-byte-array-to-string`
Because of eccentricities in the way cl-case works, the previous code
didn't quite work. This version is correct.
This abstracts functionality previously hardcoded into
ebib--fill-strings-buffer which change to simplify.

Better display: leading flags for raw and multiline values, add a
third column for the complete expansion of raw values.
Mimics `ebib-toggle-raw-current-field'.
Redisplay all strings in buffer, because changing/deleting/adding one
string could affect the display of others which include/use it.

This makes ebib--redisplay-current-string redundant, so remove it.
Adding a new field to an entry now offers completion candidates for
the name of the new field. Candidates are all fields ebib knows about
for the current dialect, plus all those undefined but used in the
current database, less all fields in the current entry.
…d-cont

Add an option `kill' to ebib-delete-field-contents. Non-nill kill
pushes the contents of the field to the kill ring. Implement
ebib-{delete,kill}-current-field-contents in terms of this function.
This reduces code duplication.
new custom variable: ebib-notes-extract-text-function. Points to a
function which should return a list of strings in the note of the
given key. ebib--extract-note-text is now just a wrapper over calling
this function. ebib-notes-extract-text-function defaults to...

new function: ebib-extract-note-text-default. Unifies the work
previously done by ebib--extract-note-text and
ebib--extract-note-text-1 into one backend function. Has sensible
defaults for notes in separate files or the same file.
Add optional argument to truncate text (as was previously always
done). As before, truncation is to `ebib-notes-display-max-lines'.
Also affects ebib-extract-note-text-default, which handles this in
most cases.

Update docstring of ebib-notes-extract-text-function to reflect this.

Add use of 'truncate option to relevant invocations in other parts of
the code.
Trying to delete the field 'external' (displayed as 'external note')
will display appropriate message ('field' is replaced by 'note'). When
`ebib-notes-storage' is set to `one-file-per-note', deletion is
affected by deleting the file -- otherwise it is not supported and an
error to this effect is raised, suggesting the user delete the note
themselves. When the `kill' argument is non-nil, the full contents of
the note (as returned by `(ebib--extract-note-text key nil)') are
pushed to the kill ring.
@joostkremers
Copy link
Owner

My apologies for the very late reply. I'm a little hesitant to add this to Ebib, because of the change that is being made to ebib--bib-find-bibtex-entries. I currently don't have a better idea, so I may change my mind, but I would like to first consider whether an alternative method can be found.

I prefer to use a face for this that is not bold.
This gives us the opportunity to return the original string when an error
happens, so that the abbrev doesn't just disappear.

This is a simple way to handle Github issue joostkremers#242.
If the user add, modifies or deletes @string abbrevs, the display of abbrev in
the current entry may be affected, and we want the user's changes to be visible
right away so that it is clear the change took effect.
joostkremers and others added 4 commits March 13, 2022 08:20
When a note file is deleted, remove the current entry's key from
ebib's internal list of which entries have notes.
ebib-delete-field-contents: keep ebib--notes-list up to date
@gkowzan
Copy link
Author

gkowzan commented Mar 14, 2022

Sure, if there's a nicer and cleaner way to achieve the same effect, then I'm all for it.

If a field value is too wide for a column, it is normally truncated. If the
column is very narrow, the visible part of the truncated string does not provide
much useful information, so it makes more sense not to display it at all.

This solves Github issue 244.
Add an optional argument `list-entries` to `ebib--bib-find-bibtex-entries`
which, if `true`, causes `ebib--bib-find-bibtex-entries` to return a list of
entry keys instead of the summary count.

An optional argument is also added to `ebib-import-entries`, which has basically
the same effect.

The reason for this change is Github PR joostkremers#241. If `ebib--bib-find-bibtex-entries`
reads a large `.bib` file, we don't want to create a long list of entry keys.
When reading only a few or even one single entry, having the entry key can be
useful.
@joostkremers
Copy link
Owner

My apologies for the host of commits that suddenly appeared in this PR... I pulled your PR but merged it onto a different commit than you had based it on, and then I pushed a commit onto your PR, so all the intermediate commits got pulled in...

You may be able to fix things by rebasing your PR on commit 600b5e3 but if not, I can simply merge my local branch into master and push that to Github.

Anyway, getting to the point: I modified your PR so that ebib--bib-find-bibtex-entries only returns a list of keys if you explicitly ask for it (and similar with ebib-import-entries). This should make ebib-biblio-import-doi work the way you intended without having to create a long list of entry keys when Ebib opens a large .bib file.

Could you please test this and see if it works? The centring doesn't seem to work reliably for me, but there may be some specific settings that interfere.

@gkowzan
Copy link
Author

gkowzan commented Mar 22, 2022

I pulled the pull request with forge and it works the same way my original pull request did. There is one issue that you probably noticed. After adding the entry, ebib seems to center on it internally but doesn't refresh the view. When I press arrow up or down, the entry above or below the new entry becomes focused. I looked into source code of ebib--goto-entry-in-index, ebib-search and several related functions and I can't figure out why it behaves that way.

@joostkremers
Copy link
Owner

I had noticed it doesn't always work, but I hadn't noticed that pressing arrow up or down moves to the entry above/below the newly added one. I'll try and take a look at it today and see if I can figure out what's going on.

@joostkremers
Copy link
Owner

Given that PR #288 provides the functionality that this PR attempts to provide, but without the additional changes that would be needed here, I'm closing this one.

@gkowzan , thanks nonetheless for providing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants