Add pre-processor directive which injects Debugger.Break()

[bjorkstromm]

Once #858 is merged, there will be use for a pre-processor directive that injects System.Diagnostics.Debugger.Break();.

This should only be injected when debug option is used.

Sorry for not using the template, but it doesn't seem to fit for new features 😄

[gep13]

@mholo65 not a problem.

That is why it says this:

<!--
TEMPLATE FOR FEATURE REQUESTS:

It's a blank slate, have fun!
-->

At the very bottom, so you are good! 😄

[bjorkstromm]

@patriksvensson when are PR's due for v0.12.0? Noticed that You've postponed this to v0.13.0, but I may have time to implement this during this week. Would be a great addition to debugging..

[devlead]

@mholo65 it's a matter of how much time we have 😉 , but please submit the PR when ready and if v.12.0 isn't released we might be able to review and merge before, otherwise it'll just go in the next release. 😄

[bjorkstromm]

ping @patriksvensson @devlead

So, I started looking into this, but I'd like some pointers on the design.

Main problem is that Processors (Cake.Core.Scripting) doesn't currently have the possibility to modify the current line. (Should they?)

I could just return true if a #break directive is processed, an then in RoslynCodeGenerator (Cake.Scripting.Roslyn) replace // #break to if (System.Diagnostics.Debugger.IsAttached) { System.Diagnostics.Debugger.Break(); }

The above solution feels kind of ad hoc, or what's your opinion? Personally, I would like to inject inside the Processor if debug is enabled but that will mess up line numbers.

[patriksvensson]

@mholo65 I'm taking a look at this now. Will get back to you asap.

[patriksvensson]

@mholo65 I think the easiest approach for now is to add an out parameter for the LineProcessor where we can return a replacement line if the line processor returned true.

string replacement = null;
if (!_lineProcessors.Any(p => p.Process(context, line, out replacement)))
{
    context.AddScriptLine(line);
}
else
{
    // Comment out processed lines to keep line data.
    context.AddScriptLine(replacement ?? string.Concat("// ", line));
}

We would have to update the existing processors to set the replacement to null for this to work though.

public override bool Process(IScriptAnalyzerContext context, string line, out string replacement)
{
    if (context == null)
    {
        throw new ArgumentNullException("context");
    }

    replacement = null;

    // More code...
}

The replacement can never exceed one line to not mess with the line numbering.

[bjorkstromm]

Thanks @patriksvensson, really appreciated! I'll try to get a PR in tomorrow!

[bjorkstromm]

@patriksvensson also, in addition to this, it would be nice if ICakeEnvironment had a readonly property IsDebug.

#break would then be converted to:
System.Diagnostics.Debugger.Break(); when ICakeEnvironment.IsDebug == true
And
// #break when ICakeEnvironment.IsDebug == false

And when #876 is implemented
if(System.Diagnostics.Debugger.IsAttached) { System.Diagnostics.Debugger.Break(); } else { System.Diagnostics.Debugger.Launch(); } when ICakeEnvironment.IsDebug == true

[patriksvensson]

@mholo65 Great suggestion. Have to think about that a little since we have plans to redesign ICakeEnvironment, but create a separate issue for it so we can track it.

[patriksvensson]

Closed via #903.