atlas-engineer / nyxt

Nyxt - the hacker's browser.
https://nyxt-browser.com/
9.88k stars 413 forks source link

Style: Leverage more of Serapeum #1185

Closed Ambrevar closed 1 year ago

Ambrevar commented 3 years ago

Serapeum has lots of cool features and it feels that Nyxt code quality could benefit from it.

I suggest we start using more of the following, in the order of the reference documentation:

Macro tools

unsplice

with-thunnk

callf

with-read-only-vars

Types

->

true

Defining Types

defunit

defunion

Binding

lret

receive (Safer.)

mvlet (Less verbose.)

Control Flow

nor

eif (Replace all ifs?)

ensure

~>

nest (Reduce indentation.)

eq*

Threads

synchronized (This one seems particularly useful for us, like in data-storage.lisp and maybe the prompt buffer.)

monitor

Iter

collecting

Conditions

ignoring (Replace all ignore-errors and some handler-case?)

Op

op (Nice shorthand for all the trivial lambdas that we have.)

Functions

eqs

partial (Replace all alex:currys?)

flip

throttle (May be useful for some of our threading stuff, like delayed serialization or downloads.)

once

Trees

walk-tree (For the GHT?)

Hash Tables

dict @ (So much nicer to read than make-hash-table and gethash!)

maphash-return

merge-tables

Box

box (We sometimes box values, but maybe always with additional data. Investigate.)

CLOS

make (Seems that Eldoc cannot show the initargs though, can we fix this?)

class-name-of (Oh, this could replace many instances of (class-name (class-of ...)).)

Packages

package-exports (Replace the manual symbol listing we are doing.)

Lists

filter-map (Replace our (delete nil...).)

in (Replace most (find ... :test #'equal) and (member ... :test #'equal).)

pop-assoc (I believe we have a few occurrences of this.)

Sequences

filter keep (Replace some remove-if-not or (remove-if (complement ...) ...).)

single (Replace our (= 1 (length ...)).)

slice (Replace our subseq that rely on the sequence length to count from the end.)

take drop (And so on. Replace some of our subseq.)

seq= (Deep sequence comparison.)

Strings

blankp

words tokens lines (Replace some of our str:split " " ... and str:split (string #\newline) ....)

fmt (Replace our (format nil ...).)

ellipsize (Replace our manual ellipsis formatting.)

Internal Definitions

local (Replace some of our deeply nested flet and labels.)

Ambrevar commented 3 years ago

Thoughts about this? I'd like to start using / converting some of these.

I'd get started with lines, words, fmt, in, blankp. class-name-of is mostly useful when imported, that is, without package prefix.

Thoughts?

jmercouris commented 3 years ago

I prefer to use standard CL whenever possible. I think converting to these constructs does not provide significant improvement to the codebase. For example, I would use (format ) whenever possible instead of str:join.

Of course if you find places in the code where you think these are better, no problem :-D.

Ambrevar commented 3 years ago

In some cases, Serapeum can replace other libraries, for instance words replace (str:split " " ...).

format is not really equivalent to str:join, in particular is renders objects, which can be undesirable (plus there is a performance penalty). The CL alternative to str:join is concatenate 'string which is very verbose :p

jmercouris commented 3 years ago

You can take a list of strings and join them with format. Format can definitely replicate the behavior of str:join :-D. I'm not saying it will be obvious or readable, but it is possible!

Ambrevar commented 3 years ago

I'd like to replace all the verbose (declaim (ftype ...)) with Serapeum's shorter and more natural equivalent.

Compare:

(declaim (ftype (function (&key (:id string) (:current-is-last-p boolean)))
                switch-buffer))

with

(-> switch-buffer (&key (:id string) (:current-is-last-p boolean)) t)

Cool? @jmercouris @aartaka

jmercouris commented 3 years ago

That's fine with me, I was never a fan of the function type declarations anyway :-D

aartaka commented 3 years ago

Cool!

Ambrevar commented 3 years ago

See https://github.com/atlas-engineer/nyxt/pull/1669.

jmercouris commented 1 year ago

Lots of good ideas in here. However, it should come up naturally as we refactor. There is no reason to force all of the code to use serapeum style everywhere.

jmercouris commented 1 year ago

In other words, we should use the best function for the job, whenever we see a place something can be replaced, lets do it, but I don't think we need an issue for it.