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.

Sign up for GitHub

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

Allow format-entry function to take optional template parameter #367

Closed
wants to merge 12 commits into from
Closed

Allow format-entry function to take optional template parameter #367

Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2a35e9d
extract internal variable logic into function
bdarcus Mar 24, 2021
d21a6c6
add suffix defcustom
bdarcus Mar 25, 2021
a3184b0
add bibtex-completion-display-formats-suffix-internal
bdarcus Mar 25, 2021
8c1a256
add suffix to format-entry
bdarcus Mar 25, 2021
bf8e3e0
remove comment
bdarcus Mar 25, 2021
82833fb
fill out docstring
bdarcus Mar 28, 2021
a6dfc52
decouple format-entry from init
bdarcus Apr 5, 2021
ba8e97f
remove 'internal' vars
bdarcus Apr 5, 2021
2397143
fix typo in the package commentary
bdarcus Apr 5, 2021
2da7c56
remove unneeded -suffix template
bdarcus Apr 5, 2021
6ed10b9
bump version to 1.1.0
bdarcus Apr 5, 2021
eb59d7a
style: indenting
bdarcus Apr 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 55 additions & 27 deletions bibtex-completion.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,17 @@ editor names."
:group 'bibtex-completion
:type '(alist :key-type symbol :value-type string))

(defcustom bibtex-completion-display-formats-suffix
'((t . "${=has-pdf=:1}${=has-note=:1} ${=type=:7}"))
"For configuring a separate suffix display.
This alist uses the same format as the display-formats template. It can be
used to render an affixation suffix or annotation, or simply a different
face for this segment of the display."
; start with this; if needed, can always turn into a more general alt template
; alist
:group 'bibtex-completion
:type '(alist :key-type symbol :value-type string))

(defvar bibtex-completion-cross-referenced-entry-types
'("proceedings" "mvproceedings" "book" "mvbook" "collection" "mvcollection")
"The list of potentially cross-referenced entry types (in lowercase).
Expand All @@ -352,6 +363,11 @@ more types can slow down resolution for large biblioraphies.")
"Stores `bibtex-completion-display-formats' together with the \"used width\" of each format string.
This is set internally.")


(defvar bibtex-completion-display-formats-suffix-internal nil
Copy link
Author

@bdarcus bdarcus Mar 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(defvar bibtex-completion-display-formats-suffix-internal nil
(defvar bibtex-completion--display-formats-suffix-internal nil

I was wanting to make these symbol name private using the -- convention, but have not since it's not currently used anywhere else in the code. Would be nice to address this at some point, as it can confusing to navigate it all without that convention.

"Stores `bibtex-completion-display-suffix-formats' together with the \"used width\" of each format string.
This is set internally.")

(defvar bibtex-completion-cache nil
"A cache storing the hash of the bibliography content and the corresponding list of entries, for each bibliography file, obtained when the bibliography was last parsed.
When the current bibliography hash is identical to the cached
Expand All @@ -364,7 +380,6 @@ bibliography file is reparsed.")
(defvar bibtex-completion-string-hash-table nil
"A hash table used for string replacements.")


(defun bibtex-completion-normalize-bibliography (&optional type)
"Return a list of bibliography file(s) in `bibtex-completion-bibliography'.
If there are org mode bibliography-files, their corresponding
Expand Down Expand Up @@ -412,24 +427,32 @@ Also sets `bibtex-completion-display-formats-internal'."
(user-error "Bibliography file %s could not be found" file)))
(bibtex-completion-normalize-bibliography))

;; Pre-calculate minimal widths needed by the format strings for
;; various entry types:
(setq bibtex-completion-display-formats-internal
Copy link
Author

@bdarcus bdarcus Mar 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also just wondering: couldn't this be set within the format-entry function? Like this?

  (let* ((processed-template
          (bibtex-completion--process-display-formats template))

That would then allow to decouple this function from the init function, so give more flexibility?

Basically, ideally I'm wanting to be able to pass in whatever arbitrary template, so I can do things like this:

(bibtex-completion-format-entry
  candidate
  (1- (frame-width))
  bibtex-actions-template-suffix)

Copy link
Author

@bdarcus bdarcus Apr 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick comparison of the existing bibtex-completion-format-entry, and my modified one, which uses the above approach.

ELISP> (benchmark-run 100
  (bibtex-actions--format-entry
   "aclu2011"
   (1- (frame-width))
   bibtex-actions-display-template))
(0.008320599000000001 0 0.0)

ELISP> (benchmark-run 100
  (bibtex-completion-format-entry
   "aclu2011"
   (1- (frame-width))))
(0.027423597 0 0.0)

They're both fast, but using this approach is even faster, though that could be because I changed mapcar to cl-loop?

So I've just pushed a commit that implements this, and then a second one that removes the now unneeded "internal" format variables.

a6dfc52
ba8e97f

Interestingly, my bibtex-actions--get-candidates function, which transforms the output of bibtex-completion-candidates, is really slow by comparison, even though the UI in selectrum seems fast.

ELISP> (benchmark-run 100 (bibtex-completion-candidates))
(1.018316332 2 0.2629074330000023)

ELISP> (benchmark-run 100 (bibtex-actions--get-candidates))
(20.117633309 43 6.017887700999999)

I ended up with a solution where I don't use the bc candidate string at all, but instead use format-entry directly there, along with adding some "invisible" metadata.

Maybe I make up elsewhere what I lose here?

(mapcar (lambda (format)
(let* ((format-string (cdr format))
(fields-width 0)
(string-width
(string-width
(s-format format-string
(lambda (field)
(setq fields-width
(+ fields-width
(string-to-number
(or (cadr (split-string field ":"))
""))))
"")))))
(-cons* (car format) format-string (+ fields-width string-width))))
bibtex-completion-display-formats)))
(bibtex-completion-process-display-format
bibtex-completion-display-formats))

(setq bibtex-completion-display-formats-suffix-internal
(bibtex-completion-process-display-format
bibtex-completion-display-formats-suffix)))

(defun bibtex-completion-process-display-format (formats)
"Pre-calculate minimal widths needed by the FORMATS strings for various entry types."
(cl-loop
for format in formats
collect
(let* ((format-string (cdr format))
(fields-width 0)
(string-width
(string-width
(s-format format-string
(lambda (field)
(setq fields-width
(+ fields-width
(string-to-number
(or (cadr (split-string field ":"))
""))))
"")))))
(-cons* (car format) format-string (+ fields-width string-width)))))

(defun bibtex-completion-clear-cache (&optional files)
"Clears FILES from cache.
Expand Down Expand Up @@ -846,17 +869,22 @@ find a PDF file."
(cl-remove-duplicates entry
:test (lambda (x y) (string= (s-downcase x) (s-downcase y)))
:key 'car :from-end t))


(defun bibtex-completion-format-entry (entry width)
(defun bibtex-completion-format-entry (entry width &optional alt-display-formats)
"Formats a BibTeX ENTRY for display in results list.
WIDTH is the width of the results list. The display format is
governed by the variable `bibtex-completion-display-formats'."
WIDTH is the width of the results list. The display format is
governed by the variable `bibtex-completion-display-formats', or
by ALT-DISPLAY-FORMATS if present."
(let* ((format
(or (assoc-string (bibtex-completion-get-value "=type=" entry)
bibtex-completion-display-formats-internal
'case-fold)
(assoc t bibtex-completion-display-formats-internal)))
(if alt-display-formats
(or (assoc-string (bibtex-completion-get-value "=type=" entry)
bibtex-completion-display-formats-suffix-internal
'case-fold)
(assoc t bibtex-completion-display-formats-suffix-internal))
(or (assoc-string (bibtex-completion-get-value "=type=" entry)
bibtex-completion-display-formats-internal
'case-fold)
(assoc t bibtex-completion-display-formats-internal))))
bdarcus marked this conversation as resolved.
Show resolved Hide resolved
(format-string (cadr format)))
(s-format
format-string
Expand Down Expand Up @@ -884,7 +912,7 @@ governed by the variable `bibtex-completion-display-formats'."
(- width (cddr format)))
0 ?\s)))))))


(defun bibtex-completion-clean-string (s)
"Remove quoting and superfluous white space from BibTeX field value in S."
(if s (replace-regexp-in-string "[\n\t ]+" " "
Expand Down