kaorahi / howm

note-taking tool on Emacs
GNU General Public License v2.0
147 stars 11 forks source link

Minor bugfixes #32

Closed nicolaisingh closed 2 months ago

nicolaisingh commented 3 months ago

Hi, these minor changes have been left idle for some time now in my local repo, submitting them in case you wish to include them in the main howm repo. Each commit is unrelated and is a different fix.

kaorahi commented 3 months ago

Hi, @nicolaisingh san. Thank you for the PR!

Due to personal reasons, I can't work on this project right away, but I'll get back to it in a week or two.

nicolaisingh commented 3 months ago

This isn't critical or anything; feel free to check it on when you are most available. 🙂

kaorahi commented 2 months ago

I've made some slight adjustments (2170577, 55cf71a, a26bece). As for 04d9286, do you see any practical issues with the doubled message? When I type C-x C-s, I strongly expect the "Wrote ..." message as a confirmation of success. If new keywords exist in the current note, the message "Wrote .../.howm-keys" can overshadow the original "Wrote ..." message. So I'd like to ensure the original message is displayed again if it becomes hidden (2170577).

This PR has been merged into the master branch (3f475a9, a5c8b37, 0a951ac, 7821b1b), but github isn't recognizing it because I did git rebase before merging. Apologies for the confusion, it might look like the PR was rejected.

nicolaisingh commented 2 months ago

Thanks @kaorahi san for taking a look at this!

do you see any practical issues with the doubled message?

It causes some confusion when I look at the *Messages* buffer; it would seem that I saved the buffer twice:

Saving file /home/nas/howm/2025/01/2025-01-22.org...
Wrote /home/nas/howm/2025/01/2025-01-22.org
Wrote /home/nas/howm/.howm-keys
Wrote /home/nas/howm/2025/01/2025-01-22.org

If the purpose of the extra call to message is to ensure that the echo area shows the current buffer being saved, I'd like to suggest putting (howm-keyword-add-current-buffer) in before-save-hook rather than in after-save-hook. The result would then be: "Wrote ..." log does not get overshadowed by "Wrote .../.howm-keys" and *Messages buffer has a cleaner output:

Saving file /home/nas/howm/2025/01/2025-01-22.org...
Wrote /home/nas/howm/.howm-keys
Wrote /home/nas/howm/2025/01/2025-01-22.org

Sample code change:


;; howm-mode.el
(defun howm-initialize-buffer ()
  (interactive)
  (when (not howm-mode)
    (error "Not howm-mode"))
  (howm-message-time "init-buf"
    (save-restriction
      (widen)
      (howm-set-configuration-for-major-mode major-mode)
      (howm-action-lock-setup)
      (howm-mode-add-font-lock)
      (howm-reminder-add-font-lock)
      (cheat-font-lock-fontify)
      ;; make-local-hook is obsolete for emacs >= 21.1.
      (howm-funcall-if-defined (make-local-hook 'after-save-hook))
      (add-hook 'before-save-hook 'howm-before-save t t) ;;   <== added
      (add-hook 'after-save-hook 'howm-after-save t t))))

(defun howm-before-save () ;;   <== added
  (when howm-mode
    (howm-keyword-add-current-buffer)))

(defun howm-after-save ()
  (when howm-mode
    ;; (howm-keyword-add-current-buffer)    <== removed
    (when howm-refresh-after-save
      (howm-initialize-buffer))
    (when (and howm-menu-refresh-after-save
               (> howm-menu-expiry-hours 0))
      (howm-menu-refresh-background))
    (run-hooks 'howm-after-save-hook)))

;; howm-common.el

(defun howm-basic-save-buffer ()
  "Silent version of `basic-save-buffer' without \"Wrote ...\" message."
  (let ((m (current-message)))
    (let ((inhibit-message t))
      (basic-save-buffer))
    ;; (unless (equal m (current-message))  ;;   <== removed
    ;;   (message "%s" m))))
    )) 
kaorahi commented 2 months ago

Thx! This sounds reasonable. I'll test it.

kaorahi commented 2 months ago

All the patches have been merged manually without using GitHub's merge button. Thanks again.