search

sharplispers / clx

a fork of crhodes' fork of danb's fork of the CLX library, an X11 client for Common Lisp
Other
114 stars 46 forks source link

Export extra accessors for DRAWABLE #111

Closed PuercoPop closed 5 years ago

PuercoPop commented 5 years ago

This partially implements the solution from https://github.com/sharplispers/clx/issues/109#issuecomment-429716844

The only thing that is missing is marking the functions as deprecated because my texinfo-fu is weak.

We could also add a :before method to emit a warning when using one of the extra accessor functions.

One thing that I found weird is that the drawable-display exports only a readable but that window-display had a corresponding accessor. Although to be honest, I don't think there is a valid use case to set the display an object is from. Should I just go ahead and make both foo-*display only :readers?

dkochmanski commented 5 years ago

This is not right, accessors should be defined only for their appropriate classes. For instance:

(defclass window (drawable) ()
  ((id :accessor window-id)))

window-id shouldn't work on pixmap object (and vice versa).

I'd make it a review, but something is wrong with github atm.

dkochmanski commented 5 years ago

This is not right, accessors should be defined only for their appropriate classes. For instance:

(defclass window (drawable) ()
  ((id :accessor window-id)))

window-id shouldn't work on pixmap object (and vice versa).

PuercoPop commented 5 years ago

@dkochmanski You are right, don't know what I was thinking when defining the extra accessor in the base class. I've updated the PR according to your feedback

PuercoPop commented 5 years ago

@dkochmanski I've included pixmap-display as well as drawable-p and window-p which where defined in the manual as well