atlas-engineer / nyxt

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

Style of class slots #2336

Closed aadcg closed 2 years ago

aadcg commented 2 years ago

I have noticed that there's a good style relative to class slots, that is usually followed in our codebase. For example:

(fullscreen-p
 nil
 :export nil
 :type boolean
 :documentation "Whether the window is displayed in fullscreen.")

The only exception is that when the value (without a docstring) is passed, then the style becomes:

(foo nil)

It would be great to propagate this style in the codebase.

aartaka commented 2 years ago

I mean, why spend a newline, if it's not strictly necessary? That's the reasoning behind the shorter version.

In the case of longer version, we mostly put initform on a separate line because, if it's on the same line as slot name, the whole definition looks terribly unbalanced.

aadcg commented 2 years ago

I didn't explain myself clearly, so I edited the first post! Above I was simply describing the "ideal" style. I think the short version is absolutely correct.

What I'm advocating for is that we strictly follow the style everywhere. As you mention, it looks unbalanced otherwise. It specially trips me when there are lots of slots and the docstrings jump around like locusts :p

aadcg commented 2 years ago

Should this be added to your style guide? I know it's still not being used everywhere.

Another question is whether we want a blank newline separating slots.

Ambrevar commented 2 years ago

Should this be added to your style guide? I know it's still not being used everywhere.

Yes! :)

Another question is whether we want a blank newline separating slots.

And no... :'( That would be overly verbose. Exception is when the class is really long, like prompter:source.

aadcg commented 2 years ago

Added.

Ambrevar commented 2 years ago

Thanks!