Add simple diagnostic output to InterfaceStubGenerator.

[nekresh]

Diagnostic messages use Visual Studio format (see:
http://blogs.msdn.com/b/msbuild/archive/2006/11/03/msbuild-visual-studio-aware-error-messages-and-message-formats.aspx)

[anaisbetts]

Does this work in Xamarin Studio on OS X?

[nekresh]

Can't install Xamarin Studio on my mac (OS X version too old) but it should just work: http://forums.xamarin.com/discussion/13117/can-xamarin-recognize-warnings-errors-produced-by-custom-build-commands
I'll test with Xamarin Studio for Windows.

[anaisbetts]

Looks like it works (on Xamarin Studio on Mac, that is). Lemme review this (or maybe @bennor can +1 it), then we'll merge it

[nekresh]

Works for me too.

I made quite a big system for only this small change but I think more warnings/errors will come as the project get bigger.
I can shave a lot of code if you think all of it isn't necessary.

[bennor]

Firstly, awesome idea! 👍 I had no idea you could do that so easily. It works perfectly (in VS at least) and it's cool to be able to double-click the warnings to go straight to the busted method. 😸

⚠️ I did have a few concerns about the approach though:

• Should we be raising warnings or errors? These methods are never going to do anything but fail hard at runtime, so wouldn't failing harder earlier with an error at build time be better?
• The DiagnosticsLogger class seems a little heavy-handed. I personally would have replaced the Dump method with just a ToString() override on the Diagnostic class, and just flushed the errors out to the console as you loop over them the first time (in InterfaceStubGenerator). It doesn't feel like there's a lot of point having a class that rolls them up just to dump them out later as a in another loop if you're not doing anything else with them. Maybe I'm missing something though?
• Lastly, just a couple of minor style things (which based on the rest of the project I think @paulcbetts agrees with), 🔥 the redundant this references (e.g. here and here, etc.) and there are a few brackets on the wrong lines - I'll try to point them out inline.

@paulcbetts that style manifesto you hinted at writing a while back would probably be helpful for people who are new to working on your stuff. 😉

[bennor]

⬆️

[anaisbetts]

that style manifesto you hinted at writing a while back would probably be helpful for people who are new to working on your stuff.

It's stuck on a machine I can't get to atm 😿

[bennor]

It's stuck on a machine I can't get to atm 😿

Ugh, that is the worst 😞

If only there were a place you could store files online and get to them from anywhere, eh? 😉

[anaisbetts]

@bennor lol this was a surprise'ish machine death, it's actually on a Time Machine backup

[bennor]

Computers. 😭

[nekresh]

I removed the this and put brackets on the right line.
I also removed the DiagnosticLogger class and its unit test.

Regarding messaging, I raised warnings as the compilation is working.
It is alright to throw errors at the developer but it will break already working code.

[nekresh]

Regarding code style, I found a Visual Studio extension that can change VS settings when a settings file is present next to a solution.
If you have a VS settings file for C# code formatting, could you please add it to refit's repository ?

[anaisbetts]

@nekresh There's a bunch of these, the problem is that nobody uses the same one. The most ubiquitous is Resharper Team Settings, but I don't use it :(

[bennor]

@paulcbetts I use ReSharper (I kind of hate it but hate writing boilerplate more, so I keep persisting with it 😞).

Anyway, I have a personal settings file that doesn't completely match the coding style but gets about 99% of the way there. I can check it in as a Team Shared file if you want? Then people who do use it won't unintentionally mangle the code.

It's just a XAML Resource Dictionary and the settings names are mostly clear enough that you should be able to validate the settings just by reading the file.

[bennor]

Other than loading LINQ queries into the condition of the loops, I think this is looking really good now.

I'm happy for it to be merged once that's taken care of. 👍

[nekresh]

Would it be interesting to generate errors for unsupported constructions like properties in refit interface ?
It wouldn't compile anyway, as the generated code doesn't care about properties, but errors are cryptic.

[bennor]

I'm not sure about that. As you say, it will generate a fairly obvious error as it is, and there's only so much we can protect people from themselves.

[anaisbetts]

Looking good. Thanks @nekresh!

[bennor]

👍