Skip to content

Conversation

@paulapatience
Copy link
Contributor

@paulapatience paulapatience commented Jun 26, 2020

RR-SET-CRTC-CONFIG was unexported, and the extension's errors (and conditions) were undefined.

Additionally, conform the order of the exports and function definitions to that of the opcodes, unexport some undefined functions, wrap some overlong lines, and clean up some comments and whitespace.

I am unsure of the naming convention for errors and conditions (and other symbols in the extensions in general). Various conventions exist in the codebase, e.g., FOO-ERROR in input.lisp, BAD-FOO in a subpackage in extensions/glx.lisp (my preference), and prefixing the errors with a common string (related to the extension name) in extensions/xvidmode.lisp (which is also the convention of extensions/randr.lisp – though not faithfully adhered to – and thus the one I followed in this PR). In #172 I removed the RR- prefix from +RR-CONNECTION+ because first, it was missing from the CONNECTION type, and second, in the RandR specification it has no prefix (unlike RRCONFIGSTATUS, for instance). However, perhaps the extensions should prefix all of their symbols (consistently) to avoid conflicting with each other and with the base library. This would be done, if at all, in a future PR, of course.

@paulapatience paulapatience force-pushed the bugfix/randr branch 2 times, most recently from 8ef475b to 6fa7bb8 Compare June 29, 2020 23:32
RR-SET-CRTC-CONFIG was unexported, and the extension's errors (and
conditions) were undefined.

Additionally, conform the order of the exports and function definitions
to that of the opcodes, unexport some undefined functions, wrap some
overlong lines, and clean up some comments and whitespace.
@dkochmanski
Copy link
Member

I think that the right convention is the one from the core module (which is documented), that is foo-error. Extensions should be adjusted to follow it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants