Thank you for review.
16 июля 2024 г., в 19:54, Philip Kaludercic <philipk@posteo.net> написал(а):
Despite
reading through the entire source code, I have no idea what you are
trying to do with this package. It would be nice to have some more
context in the Commentary section and elaborate a number of user-facing
docstrings, unless this is intended to be expert software, for people
familiar with whatever the field is.
I need to improve it. With
README.org it should be cleaner, but It should be understandable without it.
Why should we remove elisa customization group? I think there are many customization options to move it to group.
@@ -48,68 +47,61 @@
(make-llm-ollama
:embedding-model "nomic-embed-text"))
"Embeddings provider to generate embeddings."
- :group 'elisa
- :type '(sexp :validate 'cl-struct-p))
+ :type '(sexp :validate 'cl-struct-p)) ;a more specific predicate here?
It should be `llm` provider. Unfortunately there are no more specific predicate for that.
- (format "%s -f html --to plain"
- (executable-find elisa-pandoc-executable))
+ (format "%s --from html --to plain" elisa-pandoc-executable)
buffer-name t)
buffer-name)))
We should keep executable-find call here. It was feature request from users. It can be useful with per-directory PATH, like with direnv.
(defun elisa-fts-query (prompt)
"Return fts match query for PROMPT."
- (thread-last
+ (thread-last ;i belive you can do all of this with a single regular expression...
prompt
(string-trim)
(downcase)
Maybe. But I prefer to keep it readable.
@@ -1071,7 +1044,7 @@ WHERE d.rowid in %s;"
(when-let ((kind (cl-first row))
(path (cl-second row))
(text (cl-third row)))
- (pcase kind
+ (pcase kind ;is this a `pcase-exhaustive'?
("web"
(ellama-context-add-webpage-quote-noninteractive path path text))
("file"
I think it should be extendable, but for now it is not.
Best regards,
Sergey Kostyaev