Changed NoDescription to target.Name. Updated links to use HTTPS per …

[zakaluka]

…GitHub changes.

For #1993

[matthid]

Which output are you referring to? Because afaik we first print the target name and then on the next line the description.
With this change we now print the target name twice.

The relevant place is:

https://github.com/fsharp/FAKE/blob/fe316d3355e93b116e9536ad261ecc292d706554/src/app/Fake.Core.Trace/TraceListener.fs#L246-L250

Maybe in your change we just use null which will skip printing?

[matthid]

Btw: Thanks for your first contribution!

[zakaluka]

After reading your response, I see that you are right! However, at least in Ionide on Windows 7, here's what it looks like:

The description is quite prominent and helps break up the different targets and the visual flow, whereas the Start target 'XYZ' looks like any other text and is difficult to notice at a glance.

Putting a null may make more sense, but maybe possibly even better would be to emphasize the line Starting target ... instead of the description.

EDIT: The alternative may be that I just need a better console / terminal, in which case null might make the most sense.

EDIT2: Running FAKE on the latest git-bash on Windows results in no color being displayed at all (I assume because it doesn't recognize the terminal as supporting colors, even though it does)

EDIT3: For every shell I've tried on Windows 7, the coloring stays consistent as shown above. I still believe that emphasizing the target.Name makes more sense because you are guaranteed to have a Name whereas the Description is optional.

[matthid]

@zakaluka I'm open to changes there, feel free to play around and send something you are happy with :)

[zakaluka]

The latest version has 2 changes in total:

• Return null if there is no description
• If there is a description, print it to the right of the target, after a colon :

[zakaluka]

@matthid I think I've gone as far as is needed to get the output a little better. (surprisingly small amount of change)

I'm still seeing outputs of "NoDescription" in the tests, but can't figure out where they are coming from. That string doesn't occur anywhere in my codebase. If you have any insight, would love to fix it.

[matthid]

Only some very very pedantic minor things left ;)

[matthid]

Can you add a comment to OpenTag that description might be null in case it is missing (which is probably the best we can do for now, ideally we would change to string option but I don't want to make that breaking change here.

[matthid]

why not leave isNull?

Note that the current solution has a small problem when using --parallel. The "newlinecharacter might be printed after another target description (ie you have something likeStarting T1 T1Starting T2 T2: descriptionT1: description T2` followed by two newline characters). It's probably not a huge deal, but if we use the same color anyway we should probably create the string and use a single write with newline character here.

[matthid]

Regarding the tests: I think the output might come from testing the legacy fake version, which is fine (we don't really want to update that one).

[matthid]

Ok let me finish this. Again thanks!