Skip to content

Commit

Permalink
Improve help message to be more beginner friendly.
Browse files Browse the repository at this point in the history
Fix a bug which didn't show the proper command line error
fixes #2282
  • Loading branch information
matthid committed Apr 4, 2019
1 parent 9686efe commit f195d0b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
namespace Fake.Core

exception DocoptException of string
with override x.Message = sprintf "DocoptException: %s" x.Data0
with override x.Message = sprintf "%s" x.Data0

namespace Fake.Core.CommandLineParsing

Expand All @@ -12,13 +12,17 @@ open System.Text

exception private InternalException of ErrorMessageList
exception UsageException of string
with override x.Message = sprintf "UsageException: %s" x.Data0
with override x.Message = sprintf "%s" x.Data0

module private Helpers =
let raiseArgvException errlist' =
let pos = Position(null, 0L, 0L, 0L) in
let perror = ParserError(pos, null, errlist') in
raise (DocoptException(perror.ToString()))
let improveErrorText (lnNr:int64) (colNr:int64) (arg:string) (oldText:string) =
oldText.Replace(
sprintf "argv: Ln: %d Col: %d" lnNr colNr,
sprintf "Argument %d ('%s')" (colNr + 1L) arg)
let unexpectedShort = string
>> ( + ) "short option -"
>> unexpected
Expand Down Expand Up @@ -851,11 +855,14 @@ type UsageParser(usageStrings':string array, sections:(string * SafeOptions) lis
let state = ArgumentStream.create argv Map.empty
let reply = pAstParser state
let errors = ErrorMessageList.ToSortedArray(reply.Error)
let parseError = ParserError(Position("argv", int64 state.Position.ArgIndex,0L,int64 state.Position.ArgIndex), state.UserState, reply.Error)
let argIdx = int64 state.Position.ArgIndex
let parseError = ParserError(Position("argv", argIdx,0L,argIdx), state.UserState, reply.Error)
let errorText =
use sw = new System.IO.StringWriter()
parseError.WriteTo(sw)
sw.ToString()
|> Helpers.improveErrorText 0L argIdx (if argIdx >= 0L && int argIdx < argv.Length then argv.[int argIdx] else "<>")

match reply.Status = ReplyStatus.Ok, errors, state.IsEnd with
| true, [||], true -> reply.Result
| _, _ , true -> raise <| DocoptException (sprintf "errors %s: %s" (printReplyStatus reply.Status) errorText)
Expand Down
50 changes: 50 additions & 0 deletions src/app/Fake.netcore/Cli.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@ module Cli
open System
open Fake.Core.CommandLineParsing

let fakeArgsHint =
"""
General:
Fake command line is devided into runtime and script arguments.
Runtime arguments control compilation and processing of the script,
while script arguments are specific for the script or provided by
a NuGet package.
In most use cases you use the "Fake.Core.Target"-Module and therefore

This comment has been minimized.

Copy link
@agross

agross Apr 4, 2019

Here you talk about "Fake.Core.Target"-Module whereas below you talk about 'Fake.Core.Target' PACKAGE.

While I don't care about whether it's called module or package, I think one term should be used everywhere. If you choose "module" then please change this occurrence to read "Fake.Core.Target" module

This comment has been minimized.

Copy link
@matthid

matthid Apr 4, 2019

Author Member

A fake module is a NuGet package, and sadly that term is overloaded with F# module :/ Not sure how to best get out of this. We can probably change the naming everywhere in the docs if we want to

This comment has been minimized.

Copy link
@matthid

matthid Apr 4, 2019

Author Member

for starters I replaced Module in the initial section with package

inherit the correspondig command line interface. While these arguments

This comment has been minimized.

Copy link
@agross

agross Apr 4, 2019

correspondig -> corresponding

This comment has been minimized.

Copy link
@matthid

matthid Apr 4, 2019

Author Member

thanks, fixed

are not strictly part of the runtime we still show both below to
make it easier for newcomers.
-- RUNTIME ARGUMENTS SECTION --
"""

let fakeUsage =
"""
Usage:
Expand Down Expand Up @@ -37,4 +54,37 @@ Fake Build Options [build_opts]:
--fsiargs <fsiargs> [*] Arguments passed to the f# interactive.
-f, --script <script.fsx>
The script to execute (defaults to `build.fsx`).
"""

let fakeAdditionalHelp =
"""
-- SCRIPT ARGUMENTS SECTION --
THIS SECTION ONLY APPLIES IF YOU USE THE
'Fake.Core.Target' PACKAGE!

This comment has been minimized.

Copy link
@agross

agross Apr 4, 2019

Why the line break here when the rest of the output has full width?

This comment has been minimized.

Copy link
@matthid

matthid Apr 4, 2019

Author Member

Removed that

You can use the following arguments in place of `<scriptargs>`:
Usage:
fake-run --list
fake-run --version
fake-run --help | -h
fake-run [target_opts] [target <target>] [--] [<targetargs>...]
Target Module Options [target_opts]:
-t, --target <target>
Run the given target (ignored if positional
argument 'target' is given)
-e, --environment-variable <keyval> [*]
Set an environment variable. Use 'key=val'.
Consider using regular arguments, see https://fake.build/core-targets.html
-s, --single-target Run only the specified target.
-p, --parallel <num> Run parallel with the given number of tasks.
Example:
To use verbose mode (from [fake_opts]) and print all
targets use "fake -v build -- --list". Because "--list"
doesn't conflict with any of the [build_opts], you can use
"fake -v build --list"
"""
18 changes: 15 additions & 3 deletions src/app/Fake.netcore/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,15 @@ let handleAction (verboseLevel:VerboseLevel) (action:CliAction) =
traceFAKE "Paket.Core: %s" Fake.Runtime.FakeRuntimeHints.paketVersion
0
| ShowHelp ->
printf "%s" Cli.fakeArgsHint
printf "%s" Cli.fakeUsage
printfn "Hint: Run 'fake run <script.fsx> --help' to get help from your script."
printf "%s" Cli.fakeAdditionalHelp
0
| InvalidUsage str ->
eprintfn "%s" str
printfn "%s" Cli.fakeUsage
printf "%s" Cli.fakeArgsHint
printf "%s" Cli.fakeUsage
printf "%s" Cli.fakeAdditionalHelp
1
| RunOrBuild arg ->
let success = runOrBuild arg
Expand Down Expand Up @@ -334,7 +337,16 @@ let main (args:string[]) =
exitCode <- handleAction verbLevel results
with
| exn ->
printfn "Error while parsing command line, usage is:\n%s" Cli.fakeUsage
printfn "Error while parsing command line, usage is:"
printf "%s" Cli.fakeArgsHint
printf "%s" Cli.fakeUsage
printf "%s" Cli.fakeAdditionalHelp

// need to enable this as otherwise no proper error is reported
use consoleTrace =
// When silent we don't want Paket output
Paket.Logging.event.Publish
|> Observable.subscribe Paket.Logging.traceToConsole
reportExn VerboseLevel.Normal exn
exitCode <- 1
Console.OutputEncoding <- encoding
Expand Down

4 comments on commit f195d0b

@agross
Copy link

@agross agross commented on f195d0b Apr 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthid Use any name that you prefer or requires the least changes. But use it consistently!

@agross
Copy link

@agross agross commented on f195d0b Apr 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthid Use any name that you prefer or requires the least changes. But use it consistently!

@matthid
Copy link
Member Author

@matthid matthid commented on f195d0b Apr 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easier said than done ;)

@agross
Copy link

@agross agross commented on f195d0b Apr 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthid I know, I counted all the ways ".NET Core" is spelled out in the Paket documentation.

Please sign in to comment.