Drop `nyxtmuch-` prefix

[Ambrevar]

Common Lisp (unlike Emacs Lisp :p) has packages, so there is no need to prefix the symbols with nyxtmuch-.

Also the -- convention is specific to Emacs Lisp. In Common Lisp, you'd use export (or better: serapeum:export-always) instead.

It's also good practice to set the package only once at the top of the file. Switching packages in the middle confuses some applications like SLIME or SLY. Note that you can define a symbol in a non-current package by prefixing the definition name with the desired package, e.g.

(define-command nyxt::nyxtmuch-search ()
  "Open nyxtmuch with some search"
 ...)

[igoralmeida]

A consequence of (1) marginally more familiarity with emacs lisp and (2) this originally being a single .lisp file directly in nyxt's repo :)

I'm still getting to grips with how to separate nx-notmuch stuff and nyxt stuff, and ideally not even the commands like nyxtmuch-search and nyxtmuch-show should be there (but it's probably because I copied the structure from some other nx-xxxx package).

I'll use this issue to document the current (lack of) organization of the code and how I think it should be, so you can suggest a proper way to do it.

[Ambrevar]

No worries, let me know if you have any question!

As a rule of thumb, keep everything in this library package. Only very few symbols need to be defined in the nyxt package, such as global commands.

[igoralmeida]

Not sure if what I did in 95d4bac is "proper", but it does work. Most definitions are in nx-notmuch, and some commands are in nyxt. Still, I had to spray a bunch of nyxt::xxxxx calls where the functionality I [think I] need is not exported, which seems dangerous.

To finally remove all the unnecessary prefixes, I'm thinking I should split the code into separate files, maybe each a sub-package? Here's a summary of the current code:

• Main config class and its global instance (like nyxt's *browser*)
• search
  • notmuch interface
  • nyxt mode with keymap
  • nyxt command
  • buffer class
  • renderer
• show
  • same thing as above, but slightly incomplete :)
  • renderer for message bodies (with MIME handling code that should be refactored out)
  • utils
• tag
  • renderer
  • utils

Is there a need for sub-packages here? Maybe one for the notmuch interface, another for the nyxt-facing stuff (commands, buffer classes, prompter source), perhaps a third for the MIME stuff? What would be the CL way?

[Ambrevar]

I wrote a bunch of comments in the aforementioned commits, it should help.

About packages: it's up to you, really! It does not change much on the user end. CL packages are mostly meant for the developer to organize their code. If it's short and simple enough, 1 package is perfectly fine.

[Ambrevar]

Update: We've just merged https://github.com/atlas-engineer/nyxt/pull/1416, so now you can use define-command-global and you no longer need the nyxt:: prefix!

[igoralmeida]

I can no longer see the entry points for my package when using nyxt 2.0, and I suspect my package is not even being loaded, but I see no error message in the nyxt stdout or in sly. This is on latest nyxt master, 9cd6f078, and no modifications to my code here yet. My init.lisp file is simply

(load-after-system :slynk "~/.config/nyxt/my-slink.lisp")
(start-slynk)

(load-after-system :nx-notmuch)

Running the load-after-system line in sly returns nil, and calling (asdf:load-system "nx-notmuch") directly in sly gives me a backtrace saying Component "nx-notmuch" not found. What could I be missing? Not sure if relevant, but:

• I'm running the nyxt binary directly from ~/dev/nyxt/ and my package lives in ~/common-lisp/nx-notmuch/
• Calling asdf:load-system directly from a clean repl (not the slynk one) does seem to find my package, but then it fails since it can't find nyxt :)

[Ambrevar]

Sorry for the late reply, this fell under my radar :(

Since 2.0 extensions are only loaded from ~/.local/share/nyxt/extensions by default. Does it work for you if you move your extension there?
Of course you can always re-add ~/common-lisp to your source registries if you like.

We did this so that new users don't accidentally load CL packages that are not supported by Nyxt. It's also safer from a backward-compatibility perspective, because while we can safely add new source registries, we can't aford to remove them without breaking the users configuration.

[igoralmeida]

After some digging I figured out that I needed something like (push #p"~/common-lisp/nx-notmuch/" asdf:*central-registry*) in my init.lisp.

I definitely missed the notice in the changelog.

[Ambrevar]

You can also re-add the whole ~/common-lisp/ if you prefer.

I definitely missed the notice in the changelog.

Would you suggest we highlighted this more?

[igoralmeida]

I think it's fine. Since the release announcement never mentioned any such change (and rightly so, since it's not necessarily aimed at extension developers), I assumed I could just keep adding to my code. The docs I should be looking at probably did change, but I never bothered to check there after it stopped working.

[igoralmeida]

Most of the prefixes have been dropped after 4d0081f. Maybe you'd like to take a brief look and comment, @Ambrevar ? It is still very imperative, I'm probably missing some useful abstractions to avoid repeating myself. Pointers welcome.

I think after this patchset I can probably get back to https://github.com/atlas-engineer/nyxt/issues/1190 :)

Also, I realized I don't need to have global commands for everything, so I will revert some of them back to define-command, to avoid polluting M-x.

[Ambrevar]

You only need a global command for commands that may be run from unrelated buffers, such as the entry points.

[Ambrevar]

Overall, the code is looking great, congrats!

Some (nitpicky) feedback:

• Use full sentences in your docstring. In particular, end them with a period.
• Prefer first to car and rest to cdr, since they are proper English words.
• Avoid one-letter symbols or initials, use meaningful words instead, it great helps improve readability.
• You can define CSS using s-exp instead of concatenating strings manually. See cl-css. It's a Nyxt dependency, so it comes for free here.
• For nyxtmuch--format-message-body, if you don't want to keep the file open for so long, you can alex:read-file-into-string and work over the string (possibly as a stream) instead.
• The (cond ((string-equal...))) can be replaced by a more concise match.
• (format t "Got an exception: ~a~%" c): Technically a condition.
• You may want to handler-case over error instead of t to just catch errors.
• gethash has a last default argument that's returned when the key is not found, you may want to use this in format-message.
• setq is superseded by the more generic setf.
• Style: you can collate affectations like so

  (setf a a-value 
       b b-balue
       ...)