santigimeno / node-ewmh

Implementation of the Extended Window Manager Hints (EWMH)
ISC License
18 stars 1 forks source link

x11 is runtime dependency, not devDependency #2

Closed sidorares closed 10 years ago

sidorares commented 11 years ago

actually, maybe not, since everything could be referenced from client parameter...

santigimeno commented 10 years ago

In fact, I think I'll need that dependency as the eventMask is not referenced in the client or is it?

sidorares commented 10 years ago

It's not, but we can add it. What do you think?

santigimeno commented 10 years ago

I think it makes sense. Another thing that I think it could be somehow related is how do I make sure the client used by EWMH has a specific version. For example: I'm going to push some changes that needs the client to be from, at least, x11@0.5.0 Is there a way to accomplish this? Ideally if this could be configured via package.json would be great

sidorares commented 10 years ago

peerDependency ?

santigimeno commented 10 years ago

Oh, I didn't know about that. I'm going to check the npm docs. Thanks!

BTW, don't worry about adding eventMask to the client. I'll do the PR

sidorares commented 10 years ago

I'm trying to keep everything in namespace objects but probably we can make exception for core events mask and alias them directly, so one could use X.PropertyNotify

santigimeno commented 10 years ago

I'd rather do something like this: X.eventMask.PropertyNotify, so I know it is indeed a mask, but as you prefer

sidorares commented 10 years ago

Masks usually appear in big groups an I often see me typing 'eventMask' many times in the same expression

santigimeno commented 10 years ago

Yes, I understand that. I'm not totally sold on this but will do.

sidorares commented 10 years ago

Maybe make eventMask hash and helper function? X.eventMask ('PropertyNotify StructureNotify)

sidorares commented 10 years ago

Ok expose only eventMask reference :)

santigimeno commented 10 years ago

Sorry I'm not sure I understand:

X.PropertyNotify

or

X.eventMask.PropertyNotify

sidorares commented 10 years ago

The latter, as in your initial suggestion. I'm probably overcomplicating simple things. X.eventMask.PropertyNotify

santigimeno commented 10 years ago

Ok. Thanks!

santigimeno commented 10 years ago

Sent PR: https://github.com/sidorares/node-x11/pull/43