SystemCrafters / crafted-emacs

A sensible base Emacs configuration.
MIT License
739 stars 116 forks source link

Need for additional style guide - Use of free variables, unknown functions #69

Open jeastman opened 2 years ago

jeastman commented 2 years ago

I believe we are need of an additional style guide specific to Rational Emacs, which builds upon The Emacs Lisp Style Guide.

One item in particular is the use of dynamically bound variables (warned by the compiler as free-variable). Since we are forcing lexical binding (with the -*- lexical-binding: t; -*- directive), the use of dynamically bound variables would actually be considered an error as opposed to a warning. Since this is typically does not effect the end result (Emacs still works), I consider it more of a style issue than anything else.

The solution would be to use (defvar somevar) any place a dynamically bound variable (named somevar) is used.

An example from early-init.el would be:

(when (featurep 'native-compile)
  ;; Silence compiler warnings as they can be pretty disruptive
  (setq native-comp-async-report-warnings-errors nil)
  ...)

Where native-comp-async-report-warings-errors is dynamically bound.

I'm suggesting the adoption of a style where one would do the following:

(when (featurep 'native-compile)
  ;; Silence compiler warnings as they can be pretty disruptive
  (defvar native-comp-async-report-warnings-errors)
  (setq native-comp-async-report-warnings-errors nil)
  ...)

Another area where I think it would be proper to adopt a style is in the ensuring functions are declared in the context they are used. This could be handled by ensuring the appropriate module is required in the context, or providing a declare-function statement before the function is used if it could be assumed to compiler would know about it ahead of time.

An example here would be the use of straight-use-package. Assuming that we expect the Rational Emacs initialization to have occurred (which ensures the straight module is available), either a 'require' should be added:

(require 'straight)
...
(straight-use-package 'something)
....

Or the 'declare-function' could be used:

(declare-function straight-use-package "straight" (MELPA-STYLE-RECIPE &optional NO-CLONE NO-BUILD CAUSE INTERACTIVE))
...
(straight-use-package 'something)
...

I understand that Emacs is robust enough for this to "not matter", in the sense that "things will work" without this overhead. However there appears to be a mix of styles with regards to this and I think it would make sense for the project to define the style which should be used in order to provide some consistency.

bkaestner commented 2 years ago

See also #59

ajxn commented 2 years ago

If it should be a learning experience, we should probably try to add use best practice, so I think these are good suggestions. Add something to the README.org? Which then could be exported to an info file so the documentation is easy to find from within Emacs.

jeffbowman commented 1 year ago

See also: #337 #347

Maybe there are other issues related to this one which can be conflated into a single issue?