search

ccw-ide / ccw

Counterclockwise is an Eclipse plugin helping developers write Clojure code
https://github.com/laurentpetit/ccw/wiki/GoogleCodeHome
Eclipse Public License 1.0
220 stars 50 forks source link

Display exec simplifications #821

Closed laurentpetit closed 9 years ago

laurentpetit commented 9 years ago

This PR contains work on simplifying swt/ui swt/ui-sync swt/ui-sync, swt/do-ui* swt/do-ui

Write great docstrings for these functions.

arichiardi commented 9 years ago

I like the names you gave, do-ui expresses that it is a side-effecting macro, and with- that you can specify the display to run on/with...

laurentpetit commented 9 years ago

or maybe, since we're already in the swt namespace, we could just drop the ui part, and have something like this (point of view of user code):

(require '[ccw.swt :as swt])

;; macro without binding sync-exec, macro with binding with-sync-exec, fn sync-exec*
(swt/sync-exec .... macro body ... will need to reach for the display itself ...)
(swt/with-sync-exec [d (.. get the display here)] .... macro body that has the display bound to d.....)
(swt/sync-exec* (fn [display] function of the display ....))

;; macro without binding do-sync-exec, macro with binding with-sync-exec, fn sync-exec
(swt/do-sync-exec .... macro body ... will need to reach for the display itself ...)
(swt/with-sync-exec [d (.. get the display here)] .... macro body that has the display bound to d.....)
(swt/sync-exec (fn [display] function of the display ....))

;; macro without binding do-display-sync, macro with binding with-display-sync, fn display-sync
;; it makes more clear that ui thread is display thread
(swt/do-display-sync .... macro body ... will need to reach for the display itself ...)
(swt/with-display-sync [d (.. get the display here)] .... macro body that has the display bound to d.....)
(swt/display-sync (fn [display] function of the display ....))

@arichiardi @cgrand what do you think about this start?

arichiardi commented 9 years ago

yes i like the approach, this would go in ccw.swt.api I guess...

laurentpetit commented 9 years ago

yep, it would go in the API indeed. Still thinking about pros & cons of names. display-sync seems misleading, it's like we were asking for displaying some sync thing.

My current preference is for the second of my three above choices

laurentpetit commented 9 years ago

you meant ccw.api.swt I think :-)

laurentpetit commented 9 years ago

or maybe back to the original idea plus the prefix enhancements:

(swt/do-ui-sync .... macro body ... will need to reach for the display itself ...)
(swt/with-ui-sync [d (.. get the display here)] .... macro body that has the display bound to d.....)
(swt/ui-sync (fn [display] function of the display ....))
arichiardi commented 9 years ago

Yes that is the correct namespace...the second maybe has redundant do and exec.. My choice would be (the third), with the last changed?

;; macro without binding do-display-sync, macro with binding with-display-sync, fn display-sync
;; it makes more clear that ui thread is display thread
(swt/do-display-sync .... macro body ... will need to reach for the display itself ...)
(swt/with-display-sync [d (.. get the display here)] .... macro body that has the display bound to d.....)
(swt/sync-on-display (fn [display] function of the display ....))
laurentpetit commented 9 years ago

I like your idea but I'm wary of having not the same suffix for the function. Would on-display-sync be preferable than sync-on-display ? But now it looks like an event handling fn :-)

laurentpetit commented 9 years ago

ok, I think I'll let go of the with-* for the time being. All the different forms may be too confusing for users. what about either : 

(swt/do-ui-sync .... macro body ... will need to reach for the display itself ...)
(swt/ui-sync (fn [] function of the display ....))

or 

(swt/ui-sync .... macro body ... will need to reach for the display itself ...)
(swt/ui-sync* (fn [] function of the display ....))
arichiardi commented 9 years ago

The last option is display-on-sync but it sounds like you are displaying something...maybe apply-display-sync ?

arichiardi commented 9 years ago

I think either all have do, signalling side effecting fns, or none...

laurentpetit commented 9 years ago

I prefer the ! prefix for signaling side-effecting functions. Seems more idiomatic to me.

But of course, clojure itself does not always respect this convention. with-open doesn't, to give just one of plenty example.

arichiardi commented 9 years ago

Well, do usually is a macro executing a body with side effects, sometimes discarding the result value...the ! is on a proper function which cause side effects...ok is very similar...but a difference is present (my philosophy teacher said :smile:)

laurentpetit commented 9 years ago

with-open is a macro neither starting with donor ending with !;-)

doall and dorun are functions, not macros :-)

do-copy is a function :-)

do-reflect is a function :-)

io! is a macro

arichiardi commented 9 years ago

lol! :confounded:

laurentpetit commented 9 years ago

published a summary of the potential names here: https://gist.github.com/laurentpetit/5ba6ac5346fb9ee24657 and asked for help from others on twitter as well :-)

laurentpetit commented 9 years ago

updating the summary with the current sate of mind

arichiardi commented 9 years ago

Just a note: WARNING: dosync already refers to: #'clojure.core/dosync in namespace: ccw.swt, being replaced by: #'ccw.swt/dosync

laurentpetit commented 9 years ago

Oh sure, I have forgotten this one, thanks

— Envoyé avec Mailbox

On Fri, Jul 24, 2015 at 5:42 PM, Andrea Richiardi notifications@github.com wrote:

Just a note: WARNING: dosync already refers to: #'clojure.core/dosync in namespace: ccw.swt, being replaced by: #'ccw.swt/dosync

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/pull/821#issuecomment-124561986