[Mono-osx] [MonoMac] patch for MM-addin, Existing Framework

[Duane Wandless]

Throwing may be better if the framework is missing.  I can be convinced.
No symlinking for now.

The "relative path" option is only while developing and has no impact when
running.  Once installed into Contents\Frameworks the reference is always
relative to the app location.

Incorporated most of your suggestions, out of time now have to get back to
the day job.  Also added the ability for the project pad to update as
frameworks are added/deleted.  This is the same basic method Web References
uses.

Did not move the generation of the file to OnBuild.  But it is better about
not requiring a build every time.

The generated path in the dlopen call is almost correct.  Currently it is
the FULL path to the <proj>.app\Contents\Frameworks location.  Which is
incorrect.  The call to dlopen should be relative to the .app's location.

Including the new files.

Even though I disagree in part to the overuse of var I did modify this code
to use it more.

I disagree with using var when the type is known.  To me that is exactly
when you should not use var and diminishes one of C#'s benefits, strong name
typing.  At least from readability.

I agree with this summary from http://www.infoq.com/news/2008/05/CSharp-var:

Overuse of var can make source code less readable for others. It is
recommended to use var only when it is necessary, that is, when the variable
will be used to store an anonymous type or a collection of anonymous types.

For example this line is completely vague:
var ls = conf.LaunchScript;

I do not know what ls is.  What is a LaunchScript, oh if I hover over it I
see it is a FilePath.  To be me that would much more readable and useful as:
FilePath ls = conf.LaunchScript;

That makes the code clear just from a glance.  Otherwise I have to research
to determine the type.  Maybe if you limit the use to var foo = new Foo ().
 But not everywhere especially when the assignment is vague.

Duane

On Wed, Jan 26, 2011 at 6:52 PM, Michael Hutchinson <
m.j.hutchinson at gmail.com> wrote:

> On Wed, Jan 26, 2011 at 10:01 AM, Duane Wandless <duane at wandless.net>
> wrote:
> > Thanks, here is another update based on feedback.
> > Changes:
> > * renamed the menu item
> > * save to the project file as MSbuild items
> > * use CodeDOM to generate code
> > * generates MonoMacFrameworks.Initialize() in MonoMacFrameworks.cs
> > * saves to project whether relative or absolute path
> > * relies on MonoMac exporting dlopen and dlerror in
> > the MonoMac.ObjCRuntime.Dlfcn class
> > Issues:
> > * Project pad does not update when frameworks are added/deleted
> > * build must always be performed since build process modifies
> > MonoMacFrameworks.cs in wrong step
> > * UI needs more work
>
> Great, this is definitely moving quickly in the right direction :)
>
> A few more comments:
> * your patch is missing the added files - it only has the modified files.
> * please move the CodeDom code into a separate method
> * you should update the generated file before the base.OnBuild call,
> so it happens before the compilation
> * it might be nice to update the generated code immediately after any
> frameworks are added/removed. Not a must-have though.
> * you should only copy the frameworks if they change
> * the generated file should only be updated at build time if the
> project file is newer than the generated file. GetNeedsBuilding should
> check this too.
> * don't hardcode CSharpCodeProvider, you can get the appropriate
> language-specific CodeDomProvider from the DotNetProject.
> * please use var when the type is obvious from the assignment,
> especially newing, e.g. var foo = new FooBar (...);
> * property initializers can be used to make CodeDom a bit nicer.
> * if (Directory.Exists (destFramework) == true) should be if
> (Directory.Exists (destFramework))
> * wouldn't it be better to throw instead of writing a console message,
> since missing frameworks will almost certainly break an app?
> * partial modifiers are no longer needed
> * string paths can be implicitly cast to/from MD's FilePath struct,
> which neatens up path manipulation a bit
> * should there be a distinction for "local copy" frameworks, like dll
> references have? In both cases you need the loading code, but only for
> local copy does the framework need to be copied into the app AFAIK.
>
> Re. Miguel's comments, it would be good to check for missing
> frameworks when copying them and return errors in a BuildResult
> instead of letting MD present the IO exception directly.
>
> I'm not sure about the symlinking, since the apps we build are
> currently distributable without additional packaging steps, and
> symlinking would break that. Frameworks won't really hurt build
> iteration times if we fix it to to only copy frameworks when they
> change.
>
> --
> Michael Hutchinson
> http://mjhutchinson.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-osx/attachments/20110127/accccf86/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: externalFrameworks3.zip
Type: application/zip
Size: 9612 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-osx/attachments/20110127/accccf86/attachment-0001.zip 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: externalFrameworks3.diff
Type: application/octet-stream
Size: 12467 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-osx/attachments/20110127/accccf86/attachment-0001.obj