Re: [Fresco-devel] 3 patches

On Sat, Aug 31, 2002 at 08:49:36AM +0100, Neil Pilgrim wrote:
> 
> Please let me know wrt opinions of the patches and application status ;)

A few comments:

> --- Berlin/modules/Widgets/Motif/WidgetKit.cc	29 May 2002 06:49:40 -0000	1.28
> +++ Berlin/modules/Widgets/Motif/WidgetKit.cc	31 Aug 2002 07:44:43 -0000
> @@ -106,8 +106,8 @@
>  {
>    Controller_var toggle = _tools->toggle(Fresco::Graphic::_nil());
>    Fresco::ToolKit::FrameSpec s1, s2;
> -  s1.brightness(0.5); s1._d(ToolKit::inset);
> -  s2.brightness(0.5); s2._d(ToolKit::outset);
> +  s1.brightness(0.5); s1._d(Fresco::ToolKit::inset);
> +  s2.brightness(0.5); s2._d(Fresco::ToolKit::outset);

I want to know how this worked in the first place :-)

> --- Berlin/server/server.cc	6 Aug 2002 22:31:38 -0000	1.56
> +++ Berlin/server/server.cc	31 Aug 2002 07:44:48 -0000

What's going on in this next part?  I can see you reindenting, but is
it actually really consistent?  It doesn't look like it actually lines
up with other parts, though I could be wrong:

@@ somewhere, I seem to have misplaced the hunk marker
> -       DefaultPOA::default_POA(poa);
> -       Logger::log(Logger::corba) << "Default POA set up." << std::endl;
> +      DefaultPOA::default_POA(poa);
> +      Logger::log(Logger::corba) << "Default POA set up." << std::endl;
>         
> -       // ---------------------------------------------------------------
> -       // Open the Console
> -       // ---------------------------------------------------------------
> -      
> -       value = "";
> -       getopt.get("console", &value);
> -       try { Console::open(value, argc, argv, poa); }
> -       catch (const std::runtime_error &e)
> -       {
> +      // ---------------------------------------------------------------
> +      // Open the Console
> +      // ---------------------------------------------------------------
> +      value = "";
> +      getopt.get("console", &value);
> +      try { Console::open(value, argc, argv, poa); }
> +      catch (const std::runtime_error &e)
> +      {
>             std::cerr << "ERROR: Failed to open the Console \"" << value
>  		     << "\": " << e.what() << std::endl;
>  	   exit(2);

^^ Tabs!  Evil!  (This isn't the only place, either.)  (If people
would like some emacs macros to entirely and forever eliminate tabs
from their source, just ask.)

> @@ -474,9 +501,16 @@
>  	   std::cout << "Export Reference: FrescoServer="
>  		     << "corbaloc::localhost/FrescoServer" << std::endl;
>  	   Logger::log(Logger::corba) << "Corbaloc exported." << std::endl;
> -       }
> -       else if (value == "nameserver" || value.empty())
> -       {
> +           } break;
> +         case ior:
> +           {
> +	   Server_var serverRef = server->_this();
> +	   std::cout << "Export Reference: FrescoServer=" 
> +		     << orb->object_to_string(serverRef) << std::endl;
> +	   Logger::log(Logger::corba) << "IOR exported." << std::endl;
> +           } break;
> +         case nameserver:
> +           {
>  	   try
>  	   {
>  	       bind_name(orb,

Hmm.  Not exactly a problem with this patch, but I wouldn't handle the
references this way.  I'd say always try to stick it in the naming
server (with possibly a warning if you fail, but no more), and always
stick an IOR/corbaloc into an environment variable, and possibly
stdout.  (Exactly when you should print to stdout, and when you should
use a IOR vs. a corbaloc could be a bit complicated, and probably
would optimally depend on both a command line option and whether the
reference was successfully installed in the naming service.)


I'm not saying anything about the modifications to the choice widget,
because I'm too sleepy to figure out what you did, but they seem to
have design implications, so I encourage other people to comment :-)

-- Nathaniel

-- 
So let us espouse a less contested notion of truth and falsehood, even
if it is philosophically debatable (if we listen to philosophers, we
must debate everything, and there would be no end to the discussion).
  -- Serendipities, Umberto Eco

This email may be read aloud.