Feature Request: accept for buffer

bymoz089 commented 1 year ago

flyspell has a feature, which allows to store accepted words within the buffer. So if one does flyspell-correct, there is an option "Accept (buffer)". Selecting that option creates, within the current buffer, one or more lines starting (as a comment) with "LocalWords:" and behind that, are several words listed, which a user already accepted for this buffer.

With that feature it would be possible to permanently store accepted words for one file, without polluting a personal dictionary.

The functionality could work similar to already implemented jinx " [Session]" option, but it additionally stores a word on accept onto the buffers word list, and loads that list onto buffer open or jinx-mode enable.

Example of such a word list in an (random) Elisp file:

; LocalWords: barfoo foobarfoo

bymoz089 commented 1 year ago

I'm willing to implement this and the other feature request, I'd just like your approval of the ideas beforehand.

minad commented 1 year ago

Hmm, I intentionally didn't implement this feature since I don't like it. LocalWords seems like an ad-hoc invention introduced by ispell.el. in Emacs one could and should use file-local variables instead. What do you think about file-local variables? Would you also like to have that with an option to save? While LocalWords is an "established" and old standard, it isn't used much in the Emacs code base for example. I found only ~30 occurrences, which is nothing.

minad commented 1 year ago

I checked the file local variable facility and it doesn't seem perfect for this purpose since you end up with long lines. Maybe there is a way around this. But maybe this is why LocalWords has lived on for so long.

;; Local Variables:
;; jinx-local-words: ("word" "word" "word" "word" "word" "word" "word" "word" "word" "word" "word" "word" "word")
;; End:

gusbrs commented 1 year ago

If I may chime in, I also think this feature is useful. Personally, I tend to use for names. Sometimes in some documents, one or another person or company is mentioned several times, and it feels overkill to add those to the global dictionary. Perhaps this sort of need depends on the area, but for me at least (economic history) it tends to be a relatively common use case.

Edit:

I checked the file local variable facility and it doesn't seem perfect for this purpose since you end up with long lines.

Perhaps expect to use it as dir-local variable, which I think is actually a good idea, this does look a little weird as part of the document itself.

minad commented 1 year ago

This feature is definitely useful but I don't like the LocalWords special invention. I prefer if we use file local variables instead.

bymoz089 commented 1 year ago

I mentioned "LocalWords:" only to demonstrate the feature. Being somewhat compatible to already existing files would be nice, but saving accepted words to that file is more useful. Maybe a hook could solve this, for users how care about using "LocalWords:"

I was experimenting with multiline file local variables. They get read in by emacs, but updating those seems buggy.

minad commented 1 year ago

I was experimenting with multiline file local variables. They get read in by emacs, but updating those seems buggy.

That's unfortunate but we could then write our own update code.

;; Local Variables:
;; jinx-local-words: "word word word word \
;;   word word"
;; End:

bymoz089 commented 1 year ago

Here is a first implementation of the feature. It maintains a buffer-local variable jinx-local-words which gets written into .dir-locals.el upon change. * has been chosen to indicate a file/dir local accept choice.
The code changes are trivial, mostly just copying already existing functionality.

https://github.com/bymoz089/jinx https://github.com/bymoz089/jinx/commit/7a126ae64577b649572b4c7722e42a3bd998dc8d

TODO:

add two hook variables and call either one of those on accepting a local word and at loading file-local variables. Those changes should be trivial.
find a way to make the destination of this file-local variable, user selectable. While there are use cases for storing accepted words in .dir-locals.el, there are also cases where this is a bad decision (e.g. the org-roam or home directories).

minad commented 1 year ago

Looks good. We would probably need both variants, dir-local and file-local. But first, let me ask this - did you assign copyright to the FSF (or are you willing to) for your Emacs contributions? See https://github.com/minad/jinx#contributions.

bymoz089 commented 1 year ago

Not yet, the last 15 minutes I tried to figure out what to do to sign it. And while I don't care about giving up copyright of my code, I'm very sensible about giving private data. continuing tomorrow....

Edit: 2023.03.30: I requested copyright assignment forms from FSF, but got no answer, since then. I'll continue work after the copyright issue is solved, in order to not produce a solution which hinders the implementation of this feature later.

minad commented 1 year ago

Not yet, the last 15 minutes I tried to figure out what to do to sign it. And while I don't care about giving up copyright of my code, I'm very sensible about giving private data. continuing tomorrow....

Makes sense. I think I'd only need some mail address which can be associated with you for the commits. But the FSF will need your name and the other data on the form.

DamienCassou commented 1 year ago

FWIW, I'm also fan of per-file accepted words. I think it would be nice to support LocalWords so users don't have to change their files when switching the spell-checker.

While LocalWords is an "established" and old standard, it isn't used much in the Emacs code base for example. I found only ~30 occurrences, which is nothing.

True. If I count the occurrences in my ~/.emacs.d/ directory I get:

$ rg --no-ignore 'LocalWords' | wc --lines
111

It is to be noted that both the Emacs code base and my ~/.emacs.d/ directory mostly contain code. I guess spell checking is used more often in text documents than in code files.

minad commented 1 year ago

For now I just added a file-local variable jinx-local-words. But it won't be difficult to add hooks to save and load from ispell.el LocalWords.

minad commented 1 year ago

Recently I also made the behavior easily hackable via jinx--save-keys:

https://github.com/minad/jinx/blob/97ae7ce22198f7c760bbc58a95373bb9910f65dd/jinx.el#L270-L274

This way one can add actions to save as Abbrev or save as LocalWords.

DamienCassou commented 1 year ago

I wrote this code to populate jinx-local-words with ispell's "LocalWords":

(defun my/jinx-ispell-localwords ()
      "Return a string of ispell's local words.

Those are the words following `ispell-words-keyword' (usually
\"LocalWords: \") in the current buffer."
      (require 'ispell)
      (let (result)
        (save-excursion
          (goto-char (point-min))
          (while (search-forward ispell-words-keyword nil t)
            (setq result (cons (string-trim (buffer-substring-no-properties (point) (line-end-position)))
                               result))))
        (mapconcat #'identity result " ")))

(defun my/jinx-add-ispell-localwords ()
  "Add ispell's local words to `jinx-local-words'."
  (setq jinx-local-words (concat jinx-local-words (my/jinx-ispell-localwords)))
  (setq jinx--session-words (append jinx--session-words (split-string jinx-local-words))))

(add-hook 'jinx-mode-hook #'my/jinx-add-ispell-localwords)

and this code to save unknown words after the "LocalWords:" keyword:

(defun my/jinx-save-as-ispell-localword (save key word)
      "Save WORD using ispell's `ispell-words-keyword'.
If SAVE is non-nil save, otherwise format candidate given action KEY."
      (if save
          (progn
            (ispell-add-per-file-word-list word)
            (add-to-list 'jinx--session-words word)
            (setq jinx-local-words
                  (string-join
                   (sort (delete-dups
                          (cons word (split-string jinx-local-words)))
                         #'string<)
                   " ")))
        (jinx--save-action key word "File")))

(map-put! jinx--save-keys ?* #'my/jinx-save-as-ispell-localword)

Any suggestion of what I can do with this? PR, README, wiki, nothing?

minad commented 1 year ago

@DamienCassou That's great! I just enabled the wiki. Please put it there for now.

DamienCassou commented 1 year ago

Done in https://github.com/minad/jinx/wiki/Plug-with-%22LocalWords%22-from-ispell.

minad / jinx

Feature Request: accept for buffer #12