Thank you for review. > 16 июля 2024 г., в 19:54, Philip Kaludercic написал(а): > > 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. > (defcustom elisa-db-directory (file-truename > (file-name-concat > user-emacs-directory "elisa")) > "Directory for elisa database." > - :group 'elisa > - :type 'directory) > + :type 'directory) ;is it necessary that it exists? If you mean option - then yes. If you mean directory, I can create it in `elisa' package. > + (regexp-opt (mapcar #'car reps)) ;is the last one really \0 or \\0? Really. Probably impossible case, but it’s defensive programming. > +;; a number of these functions seem like something that should be added to the core of Emacs or at least a common ELPA package... > (defun elisa-dot-product (v1 v2) > "Calculate the dot produce of vectors V1 and V2." Good idea. How can we implement it? > - (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