Re: Package suggestion[nongnu]: Gnosis (γνῶσις)

[Philip Kaludercic <philipk@posteo.net>]

[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]

Thanos Apollo <public@thanosapollo.org> writes:

> Gnosis (γνῶσις), pronounced "noh-sis", meaning knowledge in Greek, is
> a [Spaced Repetition](https://en.wikipedia.org/wiki/Spaced_repetition)
> implementation for GNU Emacs.
>
> Gnosis does not implement flashcard type review sessions where the
> user rates his own answer on an arbitrary scale. Instead implements
> "note" types that require user input. Some of these note types, like
> the `MCQ`, multiple choice question, even allow for simulating
> real-life exams.
>
> Unlike other SRS implementations for GNU Emacs, gnosis does not rely on
> `org-mode`, which I've found not to be an ideal option for a database
> replacement. Instead opting for an sqlite database with git to track
> any changes made, enabling efficient data management, manipulation and
> data integrity.
>
> You can read more on my blog post here:
>   <https://thanosapollo.org/blog/gnosis-release/>
>
> This is a personal project of my mine that I made during my winter
> holidays, I have found it to be highly beneficial for my use case and,
> as a result, I decided to share it with all of you for anyone that's
> interested.
>
> Direct link to gnosis repo:
>   <https://git.thanosapollo.org/gnosis/>
>
>
> Any feedback would be appreciated, since I'm far from a programmer and
> relatively new to the GNU Emacs ecosystem.
>
> Thank you,

Here area few comments in form of a diff (this is NOT a patch):


[-- Attachment #2: Type: text/plain, Size: 19688 bytes --]

diff --git a/gnosis.el b/gnosis.el
index 7ddeb30..87e1cee 100644
--- a/gnosis.el
+++ b/gnosis.el
@@ -9,6 +9,8 @@
 
 ;; Package-Requires: ((emacs "27.2") (compat "29.1.4.2") (emacsql "20230228"))
 
+;; There is no package "emacsql" on GNU or NonGNU ELPA!
+
 ;; This program is free software; you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published by
 ;; the Free Software Foundation, either version 3 of the License, or
@@ -29,13 +31,13 @@
 ;; for GNU Emacs.
 ;;
 ;; Gnosis does not implement flashcard type review sessions where the
-;; user rates his own answer on an arbitrary scale. Instead implements
-;; "note" types that require user input. Some of these note types, like
+;; user rates his own answer on an arbitrary scale.  Instead implements
+;; "note" types that require user input.  Some of these note types, like
 ;; the MCQ, multiple choice question, even allow for simulating
 ;; real-life exams.
 ;; 
 ;; Unlike other SRS implementations for GNU Emacs, gnosis not rely on
-;; org-mode. Instead utilizes an sqlite database & git, enabling
+;; org-mode.  Instead utilizes an sqlite database & git, enabling
 ;; efficient data management, manipulation and data integrity.
 
 ;;; Code:
@@ -51,7 +53,7 @@
   :group 'external
   :prefix "gnosis-")
 
-(defcustom gnosis-dir (concat user-emacs-directory "gnosis")
+(defcustom gnosis-dir (locate-user-emacs-file "gnosis")
   "Gnosis directory."
   :type 'directory
   :group 'gnosis)
@@ -62,13 +64,13 @@
   :group 'gnosis)
 
 
-(defvar gnosis-images-dir (concat (file-name-as-directory gnosis-dir) "images")
+(defvar gnosis-images-dir (expand-file-name "images" gnosis-dir)
   "Gnosis images directory.")
 
 (defconst gnosis-db
   (if (not (file-directory-p gnosis-dir))
       (gnosis-db-init)
-    (emacsql-sqlite (concat (file-name-as-directory gnosis-dir) "gnosis.db")))
+    (emacsql-sqlite (expand-file-name "gnosis.db" gnosis-dir)))
   "Gnosis database file.")
 
 (defvar gnosis-testing nil
@@ -136,8 +138,6 @@
   "Face for next review."
   :group 'gnosis-face)
 
-
-
 (cl-defun gnosis-select (value table &optional (restrictions '1=1))
   "Select VALUE from TABLE, optionally with RESTRICTIONS.
 
@@ -264,7 +264,7 @@ SUCCESS is t when user-input is correct, else nil"
   "Replace CLOZE-CHAR with REPLACE.
 
 If FACE nil, propertize replace using `gnosis-face-correct', or
-`gnosis-face-false' when (not SUCCESS). Else use FACE value."
+`gnosis-face-false' when (not SUCCESS).  Else use FACE value."
    (goto-char (point-min))
    (search-forward cloze-char nil t)
    (replace-match (propertize replace 'face (if (not face)
@@ -329,6 +329,8 @@ Set DOWNCASE to t to downcase all input given.
 Set SPLIT to t to split all input given."
   (cl-loop with input = nil
            for response = (read-string (concat prompt " (q for quit): "))
+	   ;; Are you sure you don't want to use `read-char' here?  Or
+	   ;; `completing-read-multiple'
 	   do (if downcase (setf response (downcase response)))
            for response-parts = (if split (split-string response " ") (list response))
            if (member "q" response-parts) return (nreverse input)
@@ -397,6 +399,9 @@ When called with a prefix, unsuspends all notes for tag."
   "Suspend note(s) with specified values."
   (interactive)
   (let ((item (completing-read "Suspend by: " '("Deck" "Tag"))))
+    ;; Why use completing prompt for a two-choice option?  I think
+    ;; `yes-or-no-p' or `read-multiple-choice' would be more adequate
+    ;; and would require less custom logic.
     (pcase item
       ("Deck" (gnosis-suspend-deck))
       ("Tag" (gnosis-suspend-tag))
@@ -444,8 +449,8 @@ NOTE: If a gnosis--insert-into fails, the whole transaction will be
 
 MCQ type consists of a main `QUESTION' that is displayed to the user.
 The user will be prompted to select the correct answer from a list of
-`CHOICES'. The `CORRECT-ANSWER' should be the index of the correct
-choice in the `CHOICES' list. Each note must correspond to one `DECK'.
+`CHOICES'.  The `CORRECT-ANSWER' should be the index of the correct
+choice in the `CHOICES' list.  Each note must correspond to one `DECK'.
 
 `IMAGE' Image to display before user-input
 `SECOND-IMAGE' Image to display after user-input
@@ -470,6 +475,8 @@ TAGS: Used to organize notes
 
 Refer to `gnosis-add-note--mcq' for more."
   (let ((deck (gnosis--get-deck-name)))
+    ;; Prefer `yes-or-no-p', and let the user decide what to use with
+    ;; `use-short-answers'.
     (while (y-or-n-p (format "Add note of type `MCQ' to `%s' deck? " deck))
       (gnosis-add-note--mcq :deck deck
 			    :question (read-string "Question: ")
@@ -590,7 +597,7 @@ Example:
       {c2:Staphylococcus aureus} causes {c3:rapid} onset
       food poisoning
 
-Generates 3 cloze note types. Where the \"main\" part of the note is the full
+Generates 3 cloze note types.  Where the \"main\" part of the note is the full
 note, with the cloze(s) extracted & used as the \"answer\".
 
 One cloze note may have multiple clozes
@@ -632,7 +639,7 @@ Example:
       {c2:Staphylococcus aureus} causes {c3:rapid} onset
       food poisoning
 
-Generates 3 cloze note types. Where the \"main\" part of the note is
+Generates 3 cloze note types.  Where the \"main\" part of the note is
 the full note, with the cloze(s) extracted & used as the \"answer\".
 
 See `gnosis-add-note--cloze' for more reference."
@@ -651,6 +658,9 @@ See `gnosis-add-note--cloze' for more reference."
   (when gnosis-testing
     (unless (y-or-n-p "You are using a testing environment! Continue?")
       (error "Aborted")))
+  ;; would it make more sense to invert the control-logic using
+  ;; generic functions.  That would make it easier to extend the
+  ;; package from outside as well.
   (pcase type
     ("MCQ" (gnosis-add-note-mcq))
     ("Cloze" (gnosis-add-note-cloze))
@@ -708,6 +718,7 @@ Valid cloze formats include:
   "Compare STR1 and STR2.
 
 Compare 2 strings, ignoring case and whitespace."
+  ;; see also `string-equal-ignore-case'
   (string= (downcase (replace-regexp-in-string "\\s-" "" str1))
 	   (downcase (replace-regexp-in-string "\\s-" "" str2))))
 
@@ -718,7 +729,7 @@ DIR is the base directory path from which to start the recursive search.
 REGEX is the regular expression pattern to match the file names against.
 
 This function traverses the subdirectories of DIR recursively,
-collecting file paths that match the regular expression. The file
+collecting file paths that match the regular expression.  The file
 paths are returned as a list of strings, with each string representing
 a relative file path to DIR.
 
@@ -758,9 +769,7 @@ Optionally, add cusotm PROMPT."
 
 (defun gnosis-suspended-p (id)
   "Return t if note with ID is suspended."
-  (if (= (gnosis-get 'suspend 'review-log `(= id ,id)) 1)
-      t
-    nil))
+  (= (gnosis-get 'suspend 'review-log `(= id ,id)) 1))
 
 (defun gnosis-get-deck-due-notes (&optional deck-id)
   "Return due notes for deck, with value of DECK-ID.
@@ -788,7 +797,7 @@ DATE is a list of the form (year month day)."
      (cl-mapcan (lambda (note-id)
                   (gnosis-get-note-tags note-id))
 	        due-notes)
-     :test 'equal)))
+     :test #'equal)))
 
 
 (cl-defun gnosis-tag-prompt (&key (prompt "Selected tags") (match nil) (due nil))
@@ -816,15 +825,13 @@ Returns a list of unique tags."
   "Check if note with value of NOTE-ID for id is due for review.
 
 Check if it's suspended, and if it's due today."
-  (if (and (not (gnosis-suspended-p note-id))
-	   (gnosis-review-is-due-today-p note-id))
-      t
-    nil))
+  (and (not (gnosis-suspended-p note-id))
+       (gnosis-review-is-due-today-p note-id)))
 
 (defun gnosis-review-is-due-today-p (id)
   "Return t if note with ID is due today.
 
-This function ignores if note is suspended. Refer to
+This function ignores if note is suspended.  Refer to
 `gnosis-review-is-due-p' if you need to check for suspended value as
 well."
   (let ((next-rev (gnosis-get 'next-rev 'review-log `(= id ,id))))
@@ -867,7 +874,7 @@ Returns a list of the form ((yyyy mm dd) ef)."
      (cl-mapcan (lambda (note-id)
                   (gnosis-get-note-tags note-id))
 	        due-notes)
-     :test 'equal)))
+     :test #'equal)))
 
 (defun gnosis-review--get-offset (id)
   "Return offset for note with value of id ID."
@@ -879,7 +886,7 @@ Returns a list of the form ((yyyy mm dd) ef)."
 
 This function is used to round floating point numbers to 1 decimal,
 such as the easiness factor (ef)."
-  (/ (round (* num 100.00)) 100.00))
+  (/ (round (* num 100.00)) 100.00))	;doesn't this leave two decimals?
 
 (defun gnosis-review-new-ef (id success)
   "Return new ef for note with value of id ID.
@@ -917,8 +924,9 @@ SUCCESS is a binary value, 1 is for successful review."
 	   (answer (nth (- (gnosis-get 'answer 'notes `(= id ,id)) 1) choices))
 	   (user-choice (gnosis-mcq-answer id)))
       (if (string= answer user-choice)
-          (progn (gnosis-review--update id 1)
-		 (message "Correct!"))
+          (progn
+	    (gnosis-review--update id 1)
+	    (message "Correct!"))
 	(gnosis-review--update id 0)
 	(message "False"))
       (gnosis-display-correct-answer-mcq answer user-choice)
@@ -1014,17 +1022,18 @@ Used to reveal all clozes left with `gnosis-face-cloze-unanswered' face."
   "Commit review session on git repository.
 
 This function initializes the `gnosis-dir' as a Git repository if it is not
-already one. It then adds the gnosis.db file to the repository and commits
+already one.  It then adds the gnosis.db file to the repository and commits
 the changes with a message containing the reviewed number of notes.
 
 NOTE-NUM: The number of notes reviewed in the session."
-  (let ((git (executable-find "git"))
+  (let ((git (executable-find "git"))	;consider using `vc-git'
 	(default-directory gnosis-dir))
     (unless git
       (error "Git not found, please install git"))
     (unless (file-exists-p (concat (file-name-as-directory gnosis-dir) ".git"))
       (shell-command "git init"))
     (sit-for 0.2) ;; wait for shell command to finish
+    ;; `shell-command' is blocking anyway, so you don't need to wait
     (shell-command (concat git " add " (shell-quote-argument "gnosis.db")))
     (shell-command (concat git " commit -m "
 			   (shell-quote-argument (concat (format "Total notes for session: %d " note-num)))))
@@ -1039,13 +1048,20 @@ NOTE-NUM: The number of notes reviewed in the session."
 	(cl-loop for note in notes
 		 do (gnosis-review-note note)
 		 (setf note-count (1+ note-count))
-		 (pcase (read-char-choice "Note Action: [n]ext, [s]uspend, [e]dit, [q]uit: " '(?n ?s ?e ?q))
+		 (pcase (car (read-multiple-choice
+			      "Note actions"
+			      '((?n "next")
+				(?s "suspend")
+				(?e "edit")
+				(?q "quit"))))
 		   (?n nil)
 		   (?s (gnosis-suspend-note note))
-		   (?e (progn (gnosis-edit-note note)
-			      (recursive-edit)))
-		   (?q (progn (gnosis-review-commit note-count)
-			      (cl-return))))
+		   (?e
+		    (gnosis-edit-note note)
+		    (recursive-edit))
+		   (?q
+		    (gnosis-review-commit note-count)
+		    (cl-return)))
 		 finally (gnosis-review-commit note-count))))))
 
 
@@ -1065,6 +1081,7 @@ NOTE-NUM: The number of notes reviewed in the session."
 			   ("Total" 2)
 			   ("Decrease" 1)
 			   ("Increase" 0)))
+	;; Thy does this have to be a float?  Why not use `read-number'?
 	(new-value (float (string-to-number (read-string "New value: ")))))
     ;; error checking.
     (cond ((>= 0 new-value) (error "New value needs to be a number & higher than `0'"))
@@ -1079,7 +1096,7 @@ NOTE-NUM: The number of notes reviewed in the session."
   "Edit the contents of a note with the given ID.
 
 This function creates an Emacs Lisp buffer named *gnosis-edit* and populates it
-with the values of the note identified by the specified ID. The note values are
+with the values of the note identified by the specified ID.  The note values are
 inserted as keywords for the `gnosis-edit-update-note' function.
 
 To make changes, edit the values in the buffer, and then evaluate the
@@ -1109,6 +1126,9 @@ changes."
     (with-current-buffer (switch-to-buffer (get-buffer-create "*gnosis-edit*"))
       (gnosis-edit-mode)
       (erase-buffer)
+      ;; I feel it would be better to construct an s-expression and
+      ;; then transform it into a string using `pp-to-string' or some
+      ;; function like that.
       (insert ";;\n;; You are editing a gnosis note. DO NOT change the value of id.\n\n")
       (insert "(gnosis-edit-update-note ")
       (cl-loop for (field value) in `((id ,id)
@@ -1119,7 +1139,7 @@ changes."
 				      (extra-notes ,extra-notes)
 				      (image ,image)
 				      (second-image ,second-image))
-	       do (cond ((equal field 'id)
+	       do (cond ((eq field 'id)
 			 (insert (format (concat ":%s " (propertize "%s" 'read-only t) "\n") field value)))
 			((numberp value)
 			 (insert (format ":%s %s\n" field value)))
@@ -1127,7 +1147,7 @@ changes."
 			      (not (equal value nil)))
 			 (insert (format ":%s '%s\n" field (format "%s" (cl-loop for item in value
 										 collect (format "\"%s\"" item))))))
-			((equal value nil)
+			((nul value)
 			 (insert (format ":%s %s\n" field 'nil)))
 			(t (insert (format ":%s \"%s\"\n" field value)))))
       (delete-char -1) ;; delete extra line
@@ -1148,11 +1168,8 @@ changes."
 
 (define-derived-mode gnosis-edit-mode emacs-lisp-mode "Gnosis EDIT"
   "Gnosis Edit Mode."
-  :interactive t
-  :lighter " gnosis-edit-mode"
-  :keymap gnosis-edit-mode-map)
-
-
+  :lighter " gnosis-edit-mode")		;this is a rather long lighter...
+  
 (cl-defun gnosis-edit-update-note (&key id main options answer tags (extra-notes nil) (image nil) (second-image nil))
   "Update note with id value of ID.
 
@@ -1175,7 +1192,7 @@ SECOND-IMAGE: Image to display after user-input"
              (image . ,image)
              (second-image . ,second-image))
            when value
-           do (cond ((member field '(extra-notes image second-image))
+           do (cond ((memq field '(extra-notes image second-image))
 		     (gnosis-update 'extras `(= ,field ,value) `(= id ,id)))
 		    ((listp value)
 		     (gnosis-update 'notes `(= ,field ',value) `(= id ,id)))
@@ -1191,13 +1208,13 @@ SECOND-IMAGE: Image to display after user-input"
 ID: Identifier of the note to export.
 
 This function retrieves the fields of a note with the given ID and
-inserts them into the current buffer. Each field is represented as a
-property list entry. The following fields are exported: type, main,
+inserts them into the current buffer.  Each field is represented as a
+property list entry.  The following fields are exported: type, main,
 options, answer, tags, extra-notes, image, and second-image.
 
 The exported fields are formatted as key-value pairs with a colon,
-e.g., :field value. The fields are inserted sequentially into the
-buffer. For certain field values, like lists or nil, special
+e.g., :field value.  The fields are inserted sequentially into the
+buffer.  For certain field values, like lists or nil, special
 formatting is applied.
 
 If the value is a list, the elements are formatted as strings and
@@ -1249,11 +1266,11 @@ It then retrieves all the notes associated with the deck and exports
 them.
 
 The exported notes are formatted as an Emacs Lisp code block that can
-be evaluated to recreate the deck with its associated notes. The
+be evaluated to recreate the deck with its associated notes.  The
 resulting code is saved to a file with the provided FILENAME and a
 '.el' extension is added automatically.
 
-Each note is exported using the `gnosis-export-note` function. The
+Each note is exported using the `gnosis-export-note` function.  The
 generated code includes a call to `gnosis-define-deck` with the deck
 name and all notes formatted as nested lists"
   (interactive (list (read-string "Filename: ")))
@@ -1271,27 +1288,27 @@ name and all notes formatted as nested lists"
   "Define DECK consisting of NOTES, optionally add them as SUSPENDED.
 
 The `gnosis-define-deck` function adds a new deck with the specified
-name to `gnosis-db'. It also adds each note from the given list
-of `notes` to the deck. The function takes three optional arguments:
+name to `gnosis-db'.  It also adds each note from the given list
+of `notes` to the deck.  The function takes three optional arguments:
 `deck`, `notes`, and `suspended`.
 
-- `deck`: The name of the deck to be added. It should be provided as a
+- `deck`: The name of the deck to be added.  It should be provided as a
           symbol.
 
-- `notes`: A list containing the notes to be added to the deck. Each
+- `notes`: A list containing the notes to be added to the deck.  Each
            note should be represented as a property list with the
            following keys: `:type`, `:main`, `:options`, `:answer`
 
 - extras include :`:extra-notes`, `:tags`, `:image`, and `:second-image`.
 
 - `suspended`: An optional argument specifying whether the deck should
-               be created in a suspended state. A non-zero value
+               be created in a suspended state.  A non-zero value
                suspends the deck, while a value of 0 (default) creates
                the deck in an active state.
 
 When calling `gnosis-define-deck`, the deck is added to the Gnosis
-system by calling `gnosis-add-deck`. Each note is added to the deck
-using `gnosis-add-note-fields`. The function iterates over the list of
+system by calling `gnosis-add-deck`.  Each note is added to the deck
+using `gnosis-add-note-fields`.  The function iterates over the list of
 `notes` and extracts the necessary fields from each note's property
 list before adding them to the deck.
 
@@ -1300,6 +1317,10 @@ associated notes in `gnosis-db', ready for further processing or
 review."
   (gnosis-add-deck (symbol-name deck))
   (sit-for 0.1) ;; wait for low-spec computers to create deck
+
+  ;; I am not sure what is going on, but `sit-for' is not a robust way
+  ;; to handle asynchronous processes.  If everything is synchronous,
+  ;; there is no reason to block.
   (cl-loop for note in notes
 	   do (gnosis-add-note-fields (symbol-name deck)
 				      (plist-get note :type)
@@ -1318,6 +1339,11 @@ review."
   "Start gnosis review session."
   (interactive)
   (gnosis-db-init)
+  ;; There are a lot of string constants here that are being
+  ;; duplicated.  And as mentioned before, `completing-read' does not
+  ;; appear to be the best interface.  Keep in mind that not everyone
+  ;; uses a selecting interface like icomplete or vertico, where this
+  ;; kind of UI is pretty awkward.
   (let ((review-type (completing-read "Review: " '("Due notes"
 						   "Due notes of deck"
 						   "Due notes of specified tag(s)"
@@ -1377,7 +1403,7 @@ review."
     ;; Make sure gnosis-db is initialized
     (setf gnosis-db (emacsql-sqlite (concat (file-name-as-directory gnosis-dir) "gnosis.db"))))
   ;; Create database tables
-  (unless (= (length (emacsql gnosis-db [:select name :from sqlite-master :where (= type table)])) 6)
+  (unless (length= (emacsql gnosis-db [:select name :from sqlite-master :where (= type table)]) 6)
     ;; Enable foreign keys
     (emacsql gnosis-db "PRAGMA foreign_keys = ON")
     ;; Gnosis version
@@ -1397,12 +1423,10 @@ review."
 ;;;;;;;;;;;;;;;;;
 
 (define-derived-mode gnosis-mode special-mode "Gnosis"
-  "Gnosis Mode."
-  :interactive t
+  "Gnosis Mode."			;this could be extended
+  :lighter " gnosis-mode"
   (read-only-mode 0)
-  (display-line-numbers-mode 0)
-  :lighter " gnosis-mode")
-
+  (display-line-numbers-mode 0))
 
 (provide 'gnosis)
 ;;; gnosis.el ends here