Created attachment 122907 [details] [review]
add grab_focus

Created attachment 122908 [details] [review]
add REPL

shell_global_get_screen() - my outstanding strut patch adds a screen property
  like the stage property - I can commit just that part; I think it's more
  consistent with what we have already.

shell_global_grab_focus() - name should be shell_global_focus_stage() I think -
  grab_focus() implies it's something like XGrabKeyboard(). 

  Though do we want something more like XGrabKeyboard()? It looks like you'd
  be able to click or alt-Tab to a different window and leave the repl
  entry up. Might be nice to combine code from this and appelflap's 
  run dialog to get a "lightboxed modal" base class with:

    grabbed keyboard to the stage
    an actor over the entire stage
    the stage input area set to the whole screen

Comments on JS (a bit excessive perhaps for a quick debugging tool,
but putting some emphasis on style initially so we work one out...)

+        let repl = null;
+	this._clock.connect('button-press-event',
+	    function(o, event) {
+	    	log("showing repl");
+		if (repl == null)
+		    repl = new Repl.Repl();
+                else
+                    repl.hide();
+		repl.show();

- Hmm, maybe we want it a *little* less obvious than "click on the clock".
- Doesn't that cause you never to be able to get rid of it .. once
  it exists, another click hdies and shows again?
- Not sure how I feel about closing the variable like that ... my Java
  and Python instincts scream "that won't work", but that's a poor
  reason not to do it. Learning closure rules is good to prevent
  accidental memory leaks... still, maybe better to avoid such 
  constructs and store the repl on the panel object.

- To avoid memory leaks, it's definitely a good idea to always call
  .destroy() rather than .remove_actor() when you get rid of an actor.

+	log("prep eval");

- I'd suggest we should be somewhat conservative about random cruft
  in the log; proposal: any log statements left in need to be both
  informative and self-explanatory as to what code portion is logging them
  and why.

- Maybe the dance with replTextProps is more unclear than it's clear?
  What if you just made a global constant for the font then created the
  actors with a straight up new Clutter.Entry({ ... }) ?

+	entry.connect('activate', function (e) {
+            try {
+                me._doEval(e.get_text());
+            } catch (e) {
+               log(e);
+            }
+	});

- Why is the try catch there, rather than when you actually eval e?
  Uncaught exceptions get logged anyways, I'm pretty sure if there
  are code bugs.

  (In general if there is some place that is swallowing
  exceptions we should fix rather than ad-hoc working around it!)

Final comment:

 - Do we *have* to call it repl. I've seen the term term a long time
   from the scheme world, but never caught on to what it was abbreviating
   until you expanded it yesterday. I mentally class it with cons/car/cdr
   and other obscurity. "console"? Maybe a little grandiose of a term
   for this, but...

I've commited the shell_global_grab_focus() part of your patch,
renaming grab_focus() to focus_stage().

Obsoleted by later, more ambitious, work.


*** This bug has been marked as a duplicate of 590515 ***