From 1033458db92cafc29c9b6509c30fbc9721f68467 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:29:22 +0100 Subject: [PATCH 1/9] Update src/cli.toit Co-authored-by: Kasper Lund --- src/cli.toit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli.toit b/src/cli.toit index 04bfdf0..1ade2f8 100644 --- a/src/cli.toit +++ b/src/cli.toit @@ -367,7 +367,7 @@ abstract class Option: if short-name and not is-alpha-num-string_ short-name: throw "Invalid short option name: '$short-name'" if split-commas and not multi: - throw "--split_commas is only valid for multi options." + throw "--split-commas is only valid for multi options." if is-hidden and is-required: throw "Option can't be hidden and required." From 32e90ca6a4228098af7a1e30e43931fb7b983f8d Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:30:21 +0100 Subject: [PATCH 2/9] Update test. --- tests/options_test.toit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/options_test.toit b/tests/options_test.toit index 2c5dfff..7641249 100644 --- a/tests/options_test.toit +++ b/tests/options_test.toit @@ -116,7 +116,7 @@ test-flag: flag.parse "foo" test-bad-combos: - expect-throw "--split_commas is only valid for multi options.": + expect-throw "--split-commas is only valid for multi options.": cli.Option "foo" --split-commas expect-throw "Invalid short option name: '@'": From fa0a64e4be3fbabd2718233401a84f8d4e3449bc Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:31:16 +0100 Subject: [PATCH 3/9] Feedback. --- src/cli.toit | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cli.toit b/src/cli.toit index 9ec4b6b..936082e 100644 --- a/src/cli.toit +++ b/src/cli.toit @@ -655,7 +655,6 @@ class OptionPatterns extends Option: key: str[separator-index + 1..] } - /** A Uuid option. */ From 46b48e9257ab258dd2f598bf1024597a1b74d1a9 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:32:09 +0100 Subject: [PATCH 4/9] Change '--long-help' and '--short-help' to '--help'. (#34) When a short help is needed, the first paragraph of the full help is used. --- examples/main.toit | 24 ++--- src/cli.toit | 178 ++++++++++++++++++++++++++++---- src/help-generator_.toit | 12 +-- tests/help_test.toit | 156 ++++++++++++++-------------- tests/options_test.toit | 12 +-- tests/subcommand_test.toit.toit | 6 +- 6 files changed, 260 insertions(+), 128 deletions(-) diff --git a/examples/main.toit b/examples/main.toit index ebc75c8..ce61887 100644 --- a/examples/main.toit +++ b/examples/main.toit @@ -68,7 +68,7 @@ main arguments: // Creates a root command. // The name of the root command is not used. root-cmd := cli.Command "fleet_manager" - --long-help=""" + --help=""" This is an imaginary fleet manager for a fleet of Toit devices. It can not be used to manage the fleet, for example @@ -84,10 +84,10 @@ main arguments: create-status-command -> cli.Command: return cli.Command "status" - --short-help="Shows the status of the fleet:" + --help="Shows the status of the fleet:" --options=[ - cli.Flag "verbose" --short-name="v" --short-help="Show more details." --multi, - cli.OptionInt "max-lines" --short-help="Maximum number of lines to show." --default=10, + cli.Flag "verbose" --short-name="v" --help="Show more details." --multi, + cli.OptionInt "max-lines" --help="Maximum number of lines to show." --default=10, ] --examples=[ cli.Example "Show the status of the fleet:" --arguments="", @@ -115,7 +115,7 @@ create-device-command -> cli.Command: "dev", "thingy", ] - --long-help=""" + --help=""" Manage a particular device. Use the '--device' option to specify a specific device. Otherwise, the @@ -123,7 +123,7 @@ create-device-command -> cli.Command: """ --options=[ cli.Option "device" --short-name="d" - --short-help="The device to operate on." + --help="The device to operate on." ] device-cmd.add create-reset-command device-cmd.add create-upload-command @@ -131,7 +131,7 @@ create-device-command -> cli.Command: create-upload-command -> cli.Command: return cli.Command "upload" - --long-help=""" + --help=""" Uploads the given file to the device. Other useful information here. @@ -139,7 +139,7 @@ create-upload-command -> cli.Command: --rest=[ cli.OptionString "data" --type="file" - --short-help="The data to upload." + --help="The data to upload." --required, ] --examples=[ @@ -158,18 +158,18 @@ upload-to-device parsed/cli.Parsed: create-reset-command -> cli.Command: return cli.Command "reset" - --long-help=""" + --help=""" Resets the device. Other useful information here. """ --options=[ cli.OptionEnum "mode" ["hard", "soft"] - --short-help="The reset mode to use." + --help="The reset mode to use." --short-name="m" --required, cli.Flag "force" --short-name="f" - --short-help="Force the reset even if the device is active.", + --help="Force the reset even if the device is active.", ] --examples=[ cli.Example @@ -187,5 +187,5 @@ reset-device parsed/cli.Parsed: mode := parsed["mode"] force := parsed["force"] - print "Resetting device '$device' in $mode-mode." + print "Resetting device '$device' in $(mode)-mode." if force: print "Using the force if necessary." diff --git a/src/cli.toit b/src/cli.toit index 0a508d4..1ade2f8 100644 --- a/src/cli.toit +++ b/src/cli.toit @@ -39,7 +39,7 @@ class Command: short-help_/string? /** A longer description of the command. */ - long-help_/string? + help_/string? /** Examples of the command. */ examples_/List @@ -77,19 +77,53 @@ class Command: The $usage is usually constructed from the name and the arguments of the command, but can be provided explicitly if a different usage string is desired. - The $long-help is a longer description of the command that can span multiple lines. Use - indented lines to continue paragraphs (just like toitdoc). + The $help is a longer description of the command that can span multiple lines. Use + indented lines to continue paragraphs (just like toitdoc). The first paragraph of the + $help is used as short help, and should have meaningful content on its own. + */ + constructor name --usage/string?=null --help/string?=null --examples/List=[] \ + --aliases/List=[] --options/List=[] --rest/List=[] --subcommands/List=[] --hidden/bool=false \ + --run/Lambda?=null: + return Command.private name --usage=usage --help=help --examples=examples \ + --aliases=aliases --options=options --rest=rest --subcommands=subcommands --hidden=hidden \ + --run=run - The $short-help is a short description of the command. In most cases this help is a single - line, but it can span multiple lines/paragraphs if necessary. Use indented lines to - continue paragraphs (just like toitdoc). + /** + Deprecated. Use '--help' instead of '--short-help'. */ - constructor .name --usage/string?=null --short-help/string?=null --long-help/string?=null --examples/List=[] \ + constructor name --usage/string?=null --short-help/string --examples/List=[] \ + --aliases/List=[] --options/List=[] --rest/List=[] --subcommands/List=[] --hidden/bool=false \ + --run/Lambda?=null: + return Command.private name --usage=usage --short-help=short-help --examples=examples \ + --aliases=aliases --options=options --rest=rest --subcommands=subcommands --hidden=hidden \ + --run=run + + /** + Deprecated. Use '--help' instead of '--long-help'. + */ + constructor name --usage/string?=null --long-help/string --examples/List=[] \ + --aliases/List=[] --options/List=[] --rest/List=[] --subcommands/List=[] --hidden/bool=false \ + --run/Lambda?=null: + return Command.private name --usage=usage --help=long-help --examples=examples \ + --aliases=aliases --options=options --rest=rest --subcommands=subcommands --hidden=hidden \ + --run=run + + /** + Deprecated. Use '--help' with a meaningful first paragraph instead of '--short-help' and '--long-help'. + */ + constructor name --usage/string?=null --short-help/string --long-help/string --examples/List=[] \ + --aliases/List=[] --options/List=[] --rest/List=[] --subcommands/List=[] --hidden/bool=false \ + --run/Lambda?=null: + return Command.private name --usage=usage --short-help=short-help --help=long-help --examples=examples \ + --aliases=aliases --options=options --rest=rest --subcommands=subcommands --hidden=hidden \ + --run=run + + constructor.private .name --usage/string?=null --short-help/string?=null --help/string?=null --examples/List=[] \ --aliases/List=[] --options/List=[] --rest/List=[] --subcommands/List=[] --hidden/bool=false \ --run/Lambda?=null: usage_ = usage short-help_ = short-help - long-help_ = long-help + help_ = help examples_ = examples aliases_ = aliases options_ = options @@ -245,18 +279,38 @@ Non-rest options can be used with '--$name' or '-$short-name' (if provided). Res abstract class Option: name/string short-name/string? - short-help/string? + help/string? is-required/bool is-hidden/bool is-multi/bool should-split-commas/bool + /** Deprecated. Use '--help' instead of '--short-help'. */ + constructor name/string + --default/string?=null + --type/string="string" + --short-name/string?=null + --short-help/string + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + return OptionString name + --default=default + --type=type + --short-name=short-name + --help=short-help + --required=required + --hidden=hidden + --multi=multi + --split-commas=split-commas + /** An alias for $OptionString. */ constructor name/string --default/string?=null --type/string="string" --short-name/string?=null - --short-help/string?=null + --help/string?=null --required/bool=false --hidden/bool=false --multi/bool=false @@ -265,12 +319,15 @@ abstract class Option: --default=default --type=type --short-name=short-name - --short-help=short-help + --help=help --required=required --hidden=hidden --multi=multi --split-commas=split-commas + /** Deprecated. Use $help instead. */ + short-help -> string?: return help + /** Creates an option with the given $name. @@ -285,7 +342,7 @@ abstract class Option: The $short-name is optional and will normally be a single-character string when provided. - The $short-help is optional and is used for help output. It should be a full sentence, starting + The $help is optional and is used for help output. It should be a full sentence, starting with a capital letter and ending with a period. If $required is true, then the option must be provided. Otherwise, it is optional. @@ -299,7 +356,24 @@ abstract class Option: If $split-commas is true, then $multi must be true too. Values given to this option are then split on commas. For example, `--option a,b,c` will result in the list `["a", "b", "c"]`. */ - constructor.from-subclass .name --.short-name --.short-help --required --hidden --multi --split-commas: + constructor.from-subclass .name --.short-name --help/string? --required --hidden --multi --split-commas: + this.help = help + name = to-kebab name + is-required = required + is-hidden = hidden + is-multi = multi + should-split-commas = split-commas + if name.contains "=" or name.starts-with "no-": throw "Invalid option name: $name" + if short-name and not is-alpha-num-string_ short-name: + throw "Invalid short option name: '$short-name'" + if split-commas and not multi: + throw "--split-commas is only valid for multi options." + if is-hidden and is-required: + throw "Option can't be hidden and required." + + /** Deprecated. Use --help instead of '--short-help'. */ + constructor.from-subclass .name --.short-name --short-help/string --required --hidden --multi --split-commas: + help = short-help name = to-kebab name is-required = required is-hidden = hidden @@ -370,14 +444,30 @@ class OptionString extends Option: --.default=null --.type="string" --short-name/string?=null - --short-help/string?=null + --help/string?=null --required/bool=false --hidden/bool=false --multi/bool=false --split-commas/bool=false: if multi and default: throw "Multi option can't have default value." if required and default: throw "Option can't have default value and be required." - super.from-subclass name --short-name=short-name --short-help=short-help \ + super.from-subclass name --short-name=short-name --help=help \ + --required=required --hidden=hidden --multi=multi \ + --split-commas=split-commas + + /** Deprecated. Use '--help' instead of '--short-help'. */ + constructor name/string + --.default=null + --.type="string" + --short-name/string?=null + --short-help/string? + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + if multi and default: throw "Multi option can't have default value." + if required and default: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=short-help \ --required=required --hidden=hidden --multi=multi \ --split-commas=split-commas @@ -413,19 +503,36 @@ class OptionEnum extends Option: --.default=null --.type=(values.join "|") --short-name/string?=null - --short-help/string?=null + --help/string?=null --required/bool=false --hidden/bool=false --multi/bool=false --split-commas/bool=false: if multi and default: throw "Multi option can't have default value." if required and default: throw "Option can't have default value and be required." - super.from-subclass name --short-name=short-name --short-help=short-help \ + super.from-subclass name --short-name=short-name --help=help \ --required=required --hidden=hidden --multi=multi \ --split-commas=split-commas if default and not values.contains default: throw "Default value of '$name' is not a valid value: $default" + /** Deprecated. Use '--help' instead of '--short-help'. */ + constructor name/string .values/List + --.default=null + --.type=(values.join "|") + --short-name/string?=null + --short-help/string? + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + if multi and default: throw "Multi option can't have default value." + if required and default: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=short-help \ + --required=required --hidden=hidden --multi=multi \ + --split-commas=split-commas + if default and not values.contains default: + throw "Default value of '$name' is not a valid value: $default" is-flag: return false parse str/string --for-help-example/bool=false -> string: @@ -454,14 +561,30 @@ class OptionInt extends Option: --.default=null --.type="int" --short-name/string?=null - --short-help/string?=null + --help/string?=null + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + if multi and default: throw "Multi option can't have default value." + if required and default: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=help \ + --required=required --hidden=hidden --multi=multi \ + --split-commas=split-commas + + /** Deprecated. Use '--help' instead of '--short-help'. */ + constructor name/string + --.default=null + --.type="int" + --short-name/string?=null + --short-help/string? --required/bool=false --hidden/bool=false --multi/bool=false --split-commas/bool=false: if multi and default: throw "Multi option can't have default value." if required and default: throw "Option can't have default value and be required." - super.from-subclass name --short-name=short-name --short-help=short-help \ + super.from-subclass name --short-name=short-name --help=short-help \ --required=required --hidden=hidden --multi=multi \ --split-commas=split-commas @@ -494,13 +617,26 @@ class Flag extends Option: constructor name/string --.default=null --short-name/string?=null - --short-help/string?=null \ + --help/string?=null + --required/bool=false + --hidden/bool=false + --multi/bool=false: + if multi and default != null: throw "Multi option can't have default value." + if required and default != null: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=help \ + --required=required --hidden=hidden --multi=multi --no-split-commas + + /** Deprecated. Use '--help' instead of '--short-help'. */ + constructor name/string + --.default=null + --short-name/string?=null + --short-help/string? --required/bool=false --hidden/bool=false --multi/bool=false: if multi and default != null: throw "Multi option can't have default value." if required and default != null: throw "Option can't have default value and be required." - super.from-subclass name --short-name=short-name --short-help=short-help \ + super.from-subclass name --short-name=short-name --help=short-help \ --required=required --hidden=hidden --multi=multi --no-split-commas type -> string: diff --git a/src/help-generator_.toit b/src/help-generator_.toit index a77864a..e639feb 100644 --- a/src/help-generator_.toit +++ b/src/help-generator_.toit @@ -94,11 +94,11 @@ class HelpGenerator: /** Builds the description. - If available, the description is the $Command.long-help_, otherwise, the - $Command.short-help_ is used. If none exists, no description is built. + If available, the description is the $Command.help_, otherwise, the + (now deprecated) $Command.short-help_ is used. If none exists, no description is built. */ build-description -> none: - if help := command_.long-help_: + if help := command_.help_: ensure-vertical-space_ writeln_ (help.trim --right) else if short-help := command_.short-help_: @@ -194,7 +194,7 @@ class HelpGenerator: help-str := ? if help := subcommand.short-help_: help-str = help - else if long-help := subcommand.long-help_: + else if long-help := subcommand.help_: // Take the first paragraph (potentially multiple lines) of the long help. paragraph-index := long-help.index-of "\n\n" if paragraph-index == -1: @@ -244,7 +244,7 @@ class HelpGenerator: if not has-help-flag: options = options.copy short-name := has-short-help-flag ? null : "h" - help-flag := Flag "help" --short-name=short-name --short-help="Show help for this command." + help-flag := Flag "help" --short-name=short-name --help="Show help for this command." options.add help-flag sorted-options := options.sort: | a/Option b/Option | a.name.compare-to b.name @@ -269,7 +269,7 @@ class HelpGenerator: if not option.is-flag: option-str += " $type-str" - help-str := option.short-help or "" + help-str := option.help or "" additional-info := "" default-value := option.default needs-separator := false diff --git a/tests/help_test.toit b/tests/help_test.toit index 18f2422..586743f 100644 --- a/tests/help_test.toit +++ b/tests/help_test.toit @@ -31,21 +31,20 @@ test-combination: create-root := : | subcommands/List | cli.Command "root" --aliases=["r"] // Should not be visible. - --long-help=""" + --help=""" Root command. Two lines. """ - --short-help="Root command." // Should not be visible. --examples= subcommands.is-empty ? [ cli.Example "Example 1:" --arguments="--option1 foo rest" ]: [ cli.Example "Full example:" --arguments="sub --option1 root" ] --options=[ - cli.Option "option1" --short-help="Option 1.", + cli.Option "option1" --help="Option 1.", ] --rest= subcommands.is-empty ? [ - cli.Option "rest1" --short-help="Rest 1" --type="rest_type" --required, + cli.Option "rest1" --help="Rest 1" --type="rest_type" --required, ] : [] --subcommands=subcommands --run= subcommands.is-empty? (:: null) : null @@ -76,8 +75,7 @@ test-combination: sub := cli.Command "sub" --aliases=["sss"] - --long-help="Long sub." - --short-help="Short sub." + --help="Long sub." --examples=[ cli.Example "Sub Example 1:" --arguments="", cli.Example "Sub Example 2:" @@ -85,8 +83,8 @@ test-combination: --global-priority=5, ] --options=[ - cli.Option "option_sub1" --short-help="Option 1.", - cli.OptionInt "option_sub2" --short-help="Option 2." --default=42, + cli.Option "option_sub1" --help="Option 1.", + cli.OptionInt "option_sub2" --help="Option 2." --default=42, ] --run=:: null @@ -106,7 +104,7 @@ test-combination: Commands: help Show help for a command. - sub Short sub. + sub Long sub. Options: -h, --help Show help for this command. @@ -160,14 +158,14 @@ test-usage: cmd := cli.Command "root" --options=[ - cli.Option "option1" --short-help="Option 1." --required, - cli.OptionEnum "option2" ["bar", "baz"] --short-help="Option 2." --required, + cli.Option "option1" --help="Option 1." --required, + cli.OptionEnum "option2" ["bar", "baz"] --help="Option 2." --required, cli.Flag "optional" ] --rest=[ - cli.Option "rest1" --short-help="Rest 1." --required, - cli.Option "rest2" --short-help="Rest 2.", - cli.Option "rest3" --short-help="Rest 3." --multi, + cli.Option "rest1" --help="Rest 1." --required, + cli.Option "rest2" --help="Rest 2.", + cli.Option "rest3" --help="Rest 3." --multi, ] --run=:: unreachable @@ -182,13 +180,13 @@ test-usage: // Test different types. cmd = cli.Command "root" --options=[ - cli.Option "option7" --short-help="Option 7." --hidden, - cli.Option "option6" --short-help="Option 6.", - cli.Option "option5" --short-help="Option 5." --required --type="my_type", - cli.Flag "option4" --short-help="Option 4." --required, - cli.OptionEnum "option3" ["bar", "baz"] --short-help="Option 3." --required, - cli.OptionInt "option2" --short-help="Option 2." --required, - cli.Option "option1" --short-help="Option 1." --required, + cli.Option "option7" --help="Option 7." --hidden, + cli.Option "option6" --help="Option 6.", + cli.Option "option5" --help="Option 5." --required --type="my_type", + cli.Flag "option4" --help="Option 4." --required, + cli.OptionEnum "option3" ["bar", "baz"] --help="Option 3." --required, + cli.OptionInt "option2" --help="Option 2." --required, + cli.Option "option1" --help="Option 1." --required, ] --run=:: unreachable @@ -205,8 +203,8 @@ test-usage: cmd = cli.Command "root" --options=[ - cli.Option "option1" --short-help="Option 1." --required, - cli.Option "option2" --short-help="Option 2." --required, + cli.Option "option1" --help="Option 1." --required, + cli.Option "option2" --help="Option 2." --required, ] --run=:: unreachable @@ -222,13 +220,13 @@ test-usage: // Test the same options as rest arguments. cmd = cli.Command "root" --rest=[ - cli.Option "option9" --short-help="Option 9." --required, - cli.OptionInt "option2" --short-help="Option 2." --required, - cli.OptionEnum "option3" ["bar", "baz"] --short-help="Option 3." --required, - cli.Flag "option4" --short-help="Option 4." --required, - cli.Option "option5" --short-help="Option 5." --required --type="my_type", - cli.Option "option6" --short-help="Option 6." --required, - cli.Option "option7" --short-help="Option 7.", + cli.Option "option9" --help="Option 9." --required, + cli.OptionInt "option2" --help="Option 2." --required, + cli.OptionEnum "option3" ["bar", "baz"] --help="Option 3." --required, + cli.Flag "option4" --help="Option 4." --required, + cli.Option "option5" --help="Option 5." --required --type="my_type", + cli.Option "option6" --help="Option 6." --required, + cli.Option "option7" --help="Option 7.", ] --run=:: unreachable @@ -244,15 +242,15 @@ test-usage: cmd = cli.Command "root" --options=[ - cli.Flag "option3" --short-help="Option 3." --required, - cli.Option "option2" --short-help="Option 2.", - cli.Option "option1" --short-help="Option 1." --required, + cli.Flag "option3" --help="Option 3." --required, + cli.Option "option2" --help="Option 2.", + cli.Option "option1" --help="Option 1." --required, ] sub := cli.Command "sub" --options=[ - cli.Flag "sub_option3" --short-help="Option 3." --required, - cli.Option "sub_option2" --short-help="Option 2.", - cli.Option "sub_option1" --short-help="Option 1." --required, + cli.Flag "sub_option3" --help="Option 3." --required, + cli.Option "sub_option2" --help="Option 2.", + cli.Option "sub_option1" --help="Option 1." --required, ] --run=:: unreachable cmd.add sub @@ -341,7 +339,7 @@ test-commands: cmd = cli.Command "root" sub = cli.Command "sub" - --short-help="Subcommand." + --help="Subcommand." --run=:: unreachable cmd.add sub @@ -353,11 +351,11 @@ test-commands: expect-equals expected (build-commands.call [cmd]) sub2 := cli.Command "sub2" - --short-help="Subcommand 2." + --help="Subcommand 2." --run=:: unreachable cmd.add sub2 sub3 := cli.Command "asub3" - --short-help="Subcommand 3." + --help="Subcommand 3." --run=:: unreachable cmd.add sub3 @@ -373,7 +371,7 @@ test-commands: cmd = cli.Command "root" sub = cli.Command "sub" - --long-help=""" + --help=""" First paragraph. @@ -397,16 +395,14 @@ test-commands: --run=:: unreachable cmd.add sub sub2 = cli.Command "sub2" - --long-help="unused." - --short-help=""" + --help=""" Long shorthelp. """ --run=:: unreachable cmd.add sub2 sub3 = cli.Command "sub3" - --long-help="unused." - --short-help="Short help3." + --help="Short help3." --run=:: unreachable cmd.add sub3 @@ -422,7 +418,7 @@ test-commands: cmd = cli.Command "root" sub = cli.Command "help" - --short-help="My own help." + --help="My own help." --run=:: unreachable cmd.add sub @@ -436,7 +432,7 @@ test-commands: cmd = cli.Command "root" sub = cli.Command "sub" --aliases=["help"] - --short-help="Sub with 'help' alias." + --help="Sub with 'help' alias." --run=:: unreachable cmd.add sub @@ -462,34 +458,34 @@ test-options: // Try different flags, like --required, --short_help, --type, --default, multi. cmd := cli.Command "root" --options=[ - cli.OptionInt "option1" --short-help="Option 1." --default=42, - cli.Option "option2" --short-help="Option 2." --default="foo", - cli.OptionEnum "option3" ["bar", "baz"] --short-name="x" --short-help="Option 3." --default="bar", - cli.Flag "option4" --short-name="4" --short-help="Option 4." --default=false, - cli.Flag "option5" --short-help="Option 5." --default=true, - - cli.OptionInt "option6" --short-help="Option 6." --required, - cli.Option "option7" --short-help="Option 7." --required, - cli.OptionEnum "option8" ["bar", "baz"] --short-help="Option 8." --required, - cli.Flag "option9" --short-help="Option 9." --required, - - cli.OptionInt "option10" --short-help="Option 10." --multi, - cli.Option "option11" --short-help="Option 11." --multi, - cli.OptionEnum "option12" ["bar", "baz"] --short-help="Option 12." --multi, - cli.Flag "option13" --short-help="Option 13." --multi, - - cli.OptionInt "option14" --short-help="Option 14." --multi --required, - cli.Option "option15" --short-help="Option 15." --multi --required, - cli.OptionEnum "option16" ["bar", "baz"] --short-help="Option 16." --multi --required, - cli.Flag "option17" --short-help="Option 17." --multi --required, - - cli.OptionInt "option18" --short-help="Option 18." --type="my_int_type", - cli.Option "option19" --short-help="Option 19." --short-name="y" --type="my_string_type", - cli.OptionEnum "option20" ["bar", "baz"] --short-help="Option 20." --type="my_enum_type", - - cli.OptionInt "option21" --short-help="Option 21\nmulti_line_help.", - - cli.OptionInt "option22" --short-name="zz" --short-help="Option 22." --default=42, + cli.OptionInt "option1" --help="Option 1." --default=42, + cli.Option "option2" --help="Option 2." --default="foo", + cli.OptionEnum "option3" ["bar", "baz"] --short-name="x" --help="Option 3." --default="bar", + cli.Flag "option4" --short-name="4" --help="Option 4." --default=false, + cli.Flag "option5" --help="Option 5." --default=true, + + cli.OptionInt "option6" --help="Option 6." --required, + cli.Option "option7" --help="Option 7." --required, + cli.OptionEnum "option8" ["bar", "baz"] --help="Option 8." --required, + cli.Flag "option9" --help="Option 9." --required, + + cli.OptionInt "option10" --help="Option 10." --multi, + cli.Option "option11" --help="Option 11." --multi, + cli.OptionEnum "option12" ["bar", "baz"] --help="Option 12." --multi, + cli.Flag "option13" --help="Option 13." --multi, + + cli.OptionInt "option14" --help="Option 14." --multi --required, + cli.Option "option15" --help="Option 15." --multi --required, + cli.OptionEnum "option16" ["bar", "baz"] --help="Option 16." --multi --required, + cli.Flag "option17" --help="Option 17." --multi --required, + + cli.OptionInt "option18" --help="Option 18." --type="my_int_type", + cli.Option "option19" --help="Option 19." --short-name="y" --type="my_string_type", + cli.OptionEnum "option20" ["bar", "baz"] --help="Option 20." --type="my_enum_type", + + cli.OptionInt "option21" --help="Option 21\nmulti_line_help.", + + cli.OptionInt "option22" --short-name="zz" --help="Option 22." --default=42, ] sub := cli.Command "sub" --run=:: unreachable cmd.add sub @@ -564,12 +560,12 @@ test-options: // Test global options. cmd = cli.Command "root" --options=[ - cli.OptionInt "option1" --short-help="Option 1." --default=42, + cli.OptionInt "option1" --help="Option 1." --default=42, ] sub = cli.Command "sub" --options=[ - cli.OptionInt "option_sub1" --short-help="Option 1." --default=42, + cli.OptionInt "option_sub1" --help="Option 1." --default=42, ] --run=:: unreachable cmd.add sub @@ -590,7 +586,7 @@ test-options: cmd = cli.Command "root" --options=[ - cli.OptionInt "option1" --short-name="h" --short-help="Option 1." --default=42, + cli.OptionInt "option1" --short-name="h" --help="Option 1." --default=42, ] --run=:: unreachable expected := """ @@ -603,8 +599,8 @@ test-options: cmd = cli.Command "root" --options=[ - cli.OptionInt "option1" --short-help="Option 1." --default=42, - cli.OptionEnum "help" ["bar", "baz"] --short-help="Own help." + cli.OptionInt "option1" --help="Option 1." --default=42, + cli.OptionEnum "help" ["bar", "baz"] --help="Own help." ] --run=:: unreachable // No automatic help. Not even `-h`. @@ -624,7 +620,7 @@ test-examples: cmd := cli.Command "root" --options=[ - cli.OptionInt "option1" --short-help="Option 1." --default=42, + cli.OptionInt "option1" --help="Option 1." --default=42, ] --examples=[ cli.Example "Example 1:" --arguments="--option1=499", diff --git a/tests/options_test.toit b/tests/options_test.toit index b5a3a4b..7641249 100644 --- a/tests/options_test.toit +++ b/tests/options_test.toit @@ -18,7 +18,7 @@ test-string: expect-null option.default expect-equals "string" option.type expect-null option.short-name - expect-null option.short-help + expect-null option.help expect-not option.is-required expect-not option.is-hidden expect-not option.is-multi @@ -34,8 +34,8 @@ test-string: option = cli.Option "foo" --short-name="foo" expect-equals "foo" option.short-name - option = cli.Option "foo" --short-help="Some_help." - expect-equals "Some_help." option.short-help + option = cli.Option "foo" --help="Some_help." + expect-equals "Some_help." option.help option = cli.Option "foo" --required expect option.is-required @@ -50,12 +50,12 @@ test-string: expect option.should-split-commas option = cli.Option "foo" --short-name="f" \ - --short-help="Baz." --required --multi \ + --help="Baz." --required --multi \ --split-commas --type="some_type" expect-equals option.name "foo" expect-equals "some_type" option.type expect-equals option.short-name "f" - expect-equals option.short-help "Baz." + expect-equals option.help "Baz." expect option.is-required expect-not option.is-hidden expect option.is-multi @@ -116,7 +116,7 @@ test-flag: flag.parse "foo" test-bad-combos: - expect-throw "--split_commas is only valid for multi options.": + expect-throw "--split-commas is only valid for multi options.": cli.Option "foo" --split-commas expect-throw "Invalid short option name: '@'": diff --git a/tests/subcommand_test.toit.toit b/tests/subcommand_test.toit.toit index de45800..4bfdc5e 100644 --- a/tests/subcommand_test.toit.toit +++ b/tests/subcommand_test.toit.toit @@ -12,8 +12,8 @@ check-arguments expected/Map parsed/cli.Parsed: main: cmd := cli.Command "root" --options=[ - cli.Option "global_string" --short-name="g" --short-help="Global string." --required, - cli.Option "global_string2" --short-help="Global string2.", + cli.Option "global_string" --short-name="g" --help="Global string." --required, + cli.Option "global_string2" --help="Global string2.", ] expected := {:} @@ -21,7 +21,7 @@ main: executed-sub := false sub := cli.Command "sub1" --options=[ - cli.Option "sub_string" --short-name="s" --short-help="Sub string." --required, + cli.Option "sub_string" --short-name="s" --help="Sub string." --required, ] --run=:: | arguments | executed-sub = true From aa778f52d2924f4aa8bd24f4498e3ca81f6d4ba4 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:33:15 +0100 Subject: [PATCH 5/9] Use published fs package. --- package.lock | 7 +++++-- package.yaml | 3 ++- tests/package.lock | 7 +++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/package.lock b/package.lock index 6ad2fbd..704c56c 100644 --- a/package.lock +++ b/package.lock @@ -1,10 +1,13 @@ -sdk: ^2.0.0-alpha.100 +sdk: ^2.0.0-alpha.120 prefixes: fs: pkg-fs host: pkg-host packages: pkg-fs: - path: ../pkg-fs + url: github.com/toitlang/pkg-fs + name: fs + version: 1.0.0 + hash: c816c85022a155f37a4396455da9b27595050de1 prefixes: host: pkg-host pkg-host: diff --git a/package.yaml b/package.yaml index 297ecce..9d980aa 100644 --- a/package.yaml +++ b/package.yaml @@ -4,7 +4,8 @@ environment: sdk: ^2.0.0-alpha.100 dependencies: fs: - path: ../pkg-fs + url: github.com/toitlang/pkg-fs + version: ^1.0.0 host: url: github.com/toitlang/pkg-host version: ^1.11.0 diff --git a/tests/package.lock b/tests/package.lock index 6567fcf..aae3f28 100644 --- a/tests/package.lock +++ b/tests/package.lock @@ -1,4 +1,4 @@ -sdk: ^2.0.0-alpha.64 +sdk: ^2.0.0-alpha.120 prefixes: cli: .. host: pkg-host @@ -9,7 +9,10 @@ packages: fs: pkg-fs host: pkg-host pkg-fs: - path: ../../pkg-fs + url: github.com/toitlang/pkg-fs + name: fs + version: 1.0.0 + hash: c816c85022a155f37a4396455da9b27595050de1 prefixes: host: pkg-host pkg-host: From a59e94836edb8dea1aeff591906a9e8e4db5467f Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:35:18 +0100 Subject: [PATCH 6/9] Add more options. (#35) --- src/cli.toit | 99 +++++++++++++++++++++++++++++++++++++++++ tests/options_test.toit | 48 ++++++++++++++++++++ 2 files changed, 147 insertions(+) diff --git a/src/cli.toit b/src/cli.toit index 1ade2f8..936082e 100644 --- a/src/cli.toit +++ b/src/cli.toit @@ -2,6 +2,8 @@ // Use of this source code is governed by an MIT-style license that can be // found in the package's LICENSE file. +import uuid + import .parser_ import .utils_ import .help-generator_ @@ -533,6 +535,7 @@ class OptionEnum extends Option: --split-commas=split-commas if default and not values.contains default: throw "Default value of '$name' is not a valid value: $default" + is-flag: return false parse str/string --for-help-example/bool=false -> string: @@ -594,6 +597,102 @@ class OptionInt extends Option: return int.parse str --on-error=: throw "Invalid integer value for option '$name': '$str'." +/** +An option for patterns. + +Patterns are an extension to enums: they allow to specify a prefix to a value. +For example, a pattern `"interval:"` would allow values like + `"interval:1h"`, `"interval:30m"`, etc. + +Both '=' and ':' are allowed as separators between the prefix and the value. +*/ +class OptionPatterns extends Option: + default/string? + patterns/List + type/string + + constructor name/string .patterns/List + --.default=null + --.type=(patterns.join "|") + --short-name/string?=null + --help/string?=null + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + if multi and default: throw "Multi option can't have default value." + if required and default: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=help \ + --required=required --hidden=hidden --multi=multi \ + --split-commas=split-commas + if default: + parse_ default --on-error=: + throw "Default value of '$name' is not a valid value: $default" + + is-flag -> bool: return false + + /** + Returns the pattern that matches the given $str in a map with the pattern as key. + */ + parse str/string --for-help-example/bool=false -> any: + return parse_ str --on-error=: + throw "Invalid value for option '$name': '$str'. Valid values are: $(patterns.join ", ")." + + parse_ str/string [--on-error]: + if not str.contains ":" and not str.contains "=": + if not patterns.contains str: on-error.call + return str + + separator-index := str.index-of ":" + if separator-index < 0: separator-index = str.index-of "=" + key := str[..separator-index] + key-with-equals := "$key=" + key-with-colon := "$key:" + if not (patterns.any: it.starts-with key-with-equals or it.starts-with key-with-colon): + on-error.call + + return { + key: str[separator-index + 1..] + } + +/** +A Uuid option. +*/ +class OptionUuid extends Option: + default/uuid.Uuid? + + /** + Creates a new Uuid option. + + The $default value is null. + + The $type is set to 'uuid'. + + Ensures that values are valid Uuids. + */ + constructor name/string + --.default=null + --short-name/string?=null + --help/string?=null + --required/bool=false + --hidden/bool=false + --multi/bool=false + --split-commas/bool=false: + if multi and default: throw "Multi option can't have default value." + if required and default: throw "Option can't have default value and be required." + super.from-subclass name --short-name=short-name --help=help \ + --required=required --hidden=hidden --multi=multi \ + --split-commas=split-commas + + is-flag: return false + + type -> string: return "uuid" + + parse str/string --for-help-example/bool=false -> uuid.Uuid: + return uuid.parse str --on-error=: + throw "Invalid value for option '$name': '$str'. Expected a UUID." + + /** An option that must be a boolean value. diff --git a/tests/options_test.toit b/tests/options_test.toit index 7641249..916d20a 100644 --- a/tests/options_test.toit +++ b/tests/options_test.toit @@ -4,11 +4,14 @@ import cli import expect show * +import uuid main: test-string test-enum + test-patterns test-int + test-uuid test-flag test-bad-combos @@ -83,6 +86,30 @@ test-enum: expect-throw "Invalid value for option 'enum': 'baz'. Valid values are: foo, bar.": option.parse "baz" +test-patterns: + option := cli.OptionPatterns "pattern" ["foo", "bar:", "baz=
"] + expect-equals option.name "pattern" + expect-null option.default + expect-equals "foo|bar:|baz=
" option.type + + option = cli.OptionPatterns "pattern" ["foo", "bar:", "baz=
"] --default="bar:1h" + expect-equals "bar:1h" option.default + + value := option.parse "foo" + expect-equals "foo" value + + value = option.parse "bar:1h" + expect-structural-equals { "bar": "1h" } value + + value = option.parse "baz=neverland" + expect-structural-equals { "baz": "neverland" } value + + expect-throw "Invalid value for option 'pattern': 'baz'. Valid values are: foo, bar:, baz=
.": + option.parse "baz" + + expect-throw "Invalid value for option 'pattern': 'not-there'. Valid values are: foo, bar:, baz=
.": + option.parse "not-there" + test-int: option := cli.OptionInt "int" expect-equals option.name "int" @@ -98,6 +125,27 @@ test-int: expect-throw "Invalid integer value for option 'int': 'foo'.": option.parse "foo" +test-uuid: + option := cli.OptionUuid "uuid" + expect-equals option.name "uuid" + expect-null option.default + expect-equals "uuid" option.type + + option = cli.OptionUuid "uuid" --default=uuid.NIL + expect-equals uuid.NIL option.default + + value := option.parse "00000000-0000-0000-0000-000000000000" + expect-equals uuid.NIL value + + value = option.parse "00000000-0000-0000-0000-000000000001" + expect-equals (uuid.parse "00000000-0000-0000-0000-000000000001") value + + expect-throw "Invalid value for option 'uuid': 'foo'. Expected a UUID.": + option.parse "foo" + + expect-throw "Invalid value for option 'uuid': '00000000-0000-0000-0000-00000000000'. Expected a UUID.": + option.parse "00000000-0000-0000-0000-00000000000" + test-flag: flag := cli.Flag "flag" --default=false expect-equals flag.name "flag" From e0f43884dc5f61b6afa094aec4dec281fc2102a3 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:37:48 +0100 Subject: [PATCH 7/9] Add 'cache' support. (#36) --- .github/workflows/ci.yml | 2 +- package.lock | 18 +- package.yaml | 8 +- src/cache.toit | 464 +++++++++++++++++++++++++++++++++++++++ src/utils_.toit | 57 +++++ tests/cache_test.toit | 278 +++++++++++++++++++++++ tests/package.lock | 17 ++ tests/package.yaml | 3 + 8 files changed, 844 insertions(+), 3 deletions(-) create mode 100644 src/cache.toit create mode 100644 tests/cache_test.toit diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0693cc1..c48ac87 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: - name: Setup constants shell: bash run: | - TOIT_VERSION=v2.0.0-alpha.100 + TOIT_VERSION=v2.0.0-alpha.120 echo "TOIT_VERSION=$TOIT_VERSION" >> $GITHUB_ENV export DOWNLOAD_DIR="${{ github.workspace }}/downloads" echo "DOWNLOAD_DIR=$DOWNLOAD_DIR" >> $GITHUB_ENV diff --git a/package.lock b/package.lock index 36b1587..704c56c 100644 --- a/package.lock +++ b/package.lock @@ -1 +1,17 @@ -# Toit Lock File. +sdk: ^2.0.0-alpha.120 +prefixes: + fs: pkg-fs + host: pkg-host +packages: + pkg-fs: + url: github.com/toitlang/pkg-fs + name: fs + version: 1.0.0 + hash: c816c85022a155f37a4396455da9b27595050de1 + prefixes: + host: pkg-host + pkg-host: + url: github.com/toitlang/pkg-host + name: host + version: 1.11.0 + hash: 7e7df6ac70d98a02f232185add81a06cec0d77e8 diff --git a/package.yaml b/package.yaml index a07dda1..9d980aa 100644 --- a/package.yaml +++ b/package.yaml @@ -1,5 +1,11 @@ name: cli description: Tools, like an argument parser, to create command-line applications. - environment: sdk: ^2.0.0-alpha.100 +dependencies: + fs: + url: github.com/toitlang/pkg-fs + version: ^1.0.0 + host: + url: github.com/toitlang/pkg-host + version: ^1.11.0 diff --git a/src/cache.toit b/src/cache.toit new file mode 100644 index 0000000..874645d --- /dev/null +++ b/src/cache.toit @@ -0,0 +1,464 @@ +// Copyright (C) 2023 Toitware ApS. All rights reserved. +// Use of this source code is governed by an MIT-style license that can be +// found in the package's LICENSE file. + +import crypto.sha256 +import encoding.base64 +import encoding.json +import fs +import fs.xdg +import host.os +import host.file +import host.directory +import system +import uuid +import writer + +import .utils_ + +/** +Handles cached files. + +Typically, caches are stored in the user's home: \$(HOME)/.cache, but users can + overwrite this by setting the \$XDG_CACHE_HOME environment variable. + +To simplify testing, the environment variable '_CACHE_DIR' can be used to + override the cache directory. +*/ + +/** +A class to manage objects that can be downloaded or generated, but should + be kept alive if possible. +*/ +class Cache: + app-name/string + path/string + + /** + Creates a new cache. + + If the \$XDG_CACHE_HOME environment variable is set, the cache is located + at \$XDG_CACHE_HOME/$app-name. Otherwise, the cache will is stored + in \$(HOME)/.cache/$app-name. + */ + constructor --app-name/string: + app-name-upper := app-name.to-ascii-upper + cache-home := xdg.cache-home + return Cache --app-name=app-name --path="$cache-home/$(app-name)" + + /** + Creates a new cache using the given $path as the cache directory. + */ + constructor --.app-name --.path: + + /** + Removes the cache entry with the given $key. + */ + remove key/string -> none: + key-path := key-path_ key + if file.is-file key-path: + file.delete key-path + else if file.is-directory key-path: + directory.rmdir --recursive key-path + + /** + Whether the cache contains the given $key. + + The key can point to a file or a directory. + */ + contains key/string -> bool: + key-path := key-path_ key + return file.is-file key-path or file.is-directory key-path + + /** + Variant of $(get key [block]). + + Returns a path to the cache entry, instead of the content. + */ + get-file-path key/string [block] -> string: + key-path := key-path_ key + if file.is-directory key-path: + throw "Cache entry '$(key)' is a directory." + + if not file.is-file key-path: + file-store := FileStore_ this key + try: + block.call file-store + if not file-store.has-stored_: + throw "Generator callback didn't store anything." + finally: + file-store.close_ + + return key-path + + /** + Returns the content of the cache entry with the given $key. + + If the cache entry doesn't exist yet, calls the $block callback + to generate it. The block is called with an instance of + $FileStore, which can be used to store the value that + should be in the cache. + + Throws, if there already exists a cache entry with the given $key, but + that entry is not a file. + */ + get key/string [block] -> ByteArray: + key-path := get-file-path key block + return file.read-content key-path + + /** + Returns the path to the cached directory item with the given $key. + + If the cache entry doesn't exist yet, calls the $block callback + to generate it. The block is called with an instance of + $DirectoryStore, which must be used to store the value that + should be in the cache. + + Throws, if there already exists a cache entry with the given $key, but + that entry is a file. + */ + get-directory-path key/string [block] -> string: + key-path := key-path_ key + if file.is-file key-path: + throw "Cache entry '$(key)' is a file." + + if not file.is-directory key-path: + directory-store := DirectoryStore_ this key + try: + block.call directory-store + if not directory-store.has-stored_: + throw "Generator callback didn't store anything." + finally: + directory-store.close_ + + return key-path + + // TODO(florian): add a `delete` method. + + ensure-cache-directory_: + directory.mkdir --recursive path + + /** + Escapes the given $path so it's valid. + Escapes '\' even if the platform is Windows, where it's a valid + path separator. + If two given paths are equal, then the escaped paths are also equal. + If they are different, then the escaped paths are also different. + */ + escape-path_ path/string -> string: + if system.platform != system.PLATFORM-WINDOWS: + return path + // On Windows, we need to escape some characters. + // We use '#' as escape character. + // We will treat '/' as the folder separator, and escape '\'. + escaped-path := path.replace --all "#" "##" + // The following characters are not allowed: + // <, >, :, ", |, ?, * + // '\' and '/' would both become folder separators, so + // we escape '\' to stay unique. + // We escape them as #. + [ '<', '>', ':', '"', '|', '?', '*', '\\' ].do: + escaped-path = escaped-path.replace --all + string.from-rune it + "#$(%02X it)" + if escaped-path.ends-with " " or escaped-path.ends-with ".": + // Windows doesn't allow files to end with a space or a dot. + // Add a suffix to make it valid. + // Note that this still guarantees uniqueness, because + // a space would normally not be escaped. + escaped-path = "$escaped-path#20" + return escaped-path + + key-path_ key/string -> string: + if system.platform == system.PLATFORM-WINDOWS and key.size > 100: + // On Windows we shorten the path so it doesn't run into the 260 character limit. + sha := sha256.Sha256 + sha.add key + key = "$(base64.encode --url-mode sha.get)" + + return "$(path)/$(escape-path_ key)" + + with-tmp-directory_ key/string?=null [block]: + ensure-cache-directory_ + prefix := ? + if key and system.platform != system.PLATFORM-WINDOWS: + // On Windows don't try to create long prefixes as paths are limited to 260 characters. + escaped-key := escape-path_ key + escaped-key = escaped-key.replace --all "/" "_" + prefix = "$(path)/$(escaped-key)-" + else: + prefix = "$(path)/tmp-" + + tmp-dir := directory.mkdtemp prefix + try: + block.call tmp-dir + finally: + // It's legal for the block to (re)move the directory. + if file.is-directory tmp-dir: + directory.rmdir --recursive tmp-dir + +/** +An interface to store a file in the cache. + +An instance of this class is provided to callers of the cache's get methods + when the key doesn't exist yet. The caller must then call one of the store + methods to fill the cache. +*/ +interface FileStore: + key -> string + + /** + Creates a temporary directory that is on the same file system as the cache. + As such, it is suitable for a $move call. + + Calls the given $block with the path as argument. + The temporary directory is deleted after the block returns. + */ + with-tmp-directory [block] + + /** + Saves the given $bytes as the content of $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + save bytes/ByteArray + + /** + Calls the given $block with a $writer.Writer. + + The $block must write its chunks to the writer. + The writer is closed after the block returns. + */ + save-via-writer [block] + + /** + Copies the content of $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + copy path/string + + /** + Moves the file at $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + move path/string + + // TODO(florian): add "download" method. + // download url/string --compressed/bool=false --path/string="": + +/** +An interface to store a directory in the cache. + +An instance of this class is provided to callers of the cache's get methods + when the key doesn't exist yet. The caller must then call one of the store + methods to fill the cache. +*/ +interface DirectoryStore: + key -> string + + /** + Creates a temporary directory that is on the same file system as the cache. + As such, it is suitable for a $move call. + + Calls the given $block with the path as argument. + The temporary directory is deleted after the block returns. + */ + with-tmp-directory [block] + + /** + Copies the content of the directory $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + copy path/string + + /** + Moves the directory at $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + move path/string + + // TODO(florian): add "download" method. + // Must be a tar, tar.gz, tgz, or zip. + // download url/string --path/string="": + + +class FileStore_ implements FileStore: + cache_/Cache + key/string + has-stored_/bool := false + is-closed_/bool := false + + constructor .cache_ .key: + + close_: is-closed_ = true + + /** + Creates a temporary directory that is on the same file system as the cache. + As such, it is suitable for a $move call. + + Calls the given $block with the path as argument. + The temporary directory is deleted after the block returns. + */ + with-tmp-directory [block]: + cache_.with-tmp-directory_ block + + /** + Saves the given $bytes as the content of $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + save bytes/ByteArray: + store_: | file-path/string | + file.write-content bytes --path=file-path + + /** + Calls the given $block with a $writer.Writer. + + The $block must write its chunks to the writer. + The writer is closed after the block returns. + */ + save-via-writer [block]: + store_: | file-path/string | + stream := file.Stream.for-write file-path + w := writer.Writer stream + try: + block.call w + finally: + w.close + + /** + Copies the content of $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + copy path/string: + store_: | file-path/string | + copy-file_ --source=path --target=file-path + + /** + Moves the file at $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + move path/string: + if has-stored_: throw "Already saved content for key: $key" + if is-closed_: throw "FileStore is closed" + + store_: | file-path/string | + // TODO(florian): we should be able to test whether the rename should succeed. + exception := catch: file.rename path file-path + if not exception: continue.store_ + // We assume that the files weren't on the same file system. + copy-file_ --source=path --target=file-path + + store_ [block] -> none: + if has-stored_: throw "Already saved content for key: $key" + if is-closed_: throw "FileStore is closed" + + // Save files into a temporary file first, then rename it to the final + // location. + cache_.with-tmp-directory_ key: | tmp-dir | + tmp-path := "$tmp-dir/content" + block.call tmp-path + key-path := cache_.key-path_ key + key-dir := fs.dirname key-path + directory.mkdir --recursive key-dir + atomic-move-file_ tmp-path key-path + + has-stored_ = true + +class DirectoryStore_ implements DirectoryStore: + cache_/Cache + key/string + has-stored_/bool := false + is-closed_/bool := false + + constructor .cache_ .key: + + close_: is-closed_ = true + + /** + Creates a temporary directory that is on the same file system as the cache. + As such, it is suitable for a $move call. + + Calls the given $block with the path as argument. + The temporary directory is deleted after the block returns. + */ + with-tmp-directory [block]: + cache_.with-tmp-directory_ block + + /** + Copies the content of the directory $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + copy path/string: + store_: | dir-path/string | + copy-directory --source=path --target=dir-path + + /** + Moves the directory at $path to the cache under $key. + + If the key already exists, the generated content is dropped. + This can happen if two processes try to access the cache at the same time. + */ + move path/string: + store_: | dir-path/string | + // TODO(florian): we should be able to test whether the rename should succeed. + exception := catch: file.rename path dir-path + if not exception: continue.store_ + // We assume that the files weren't on the same file system. + copy-directory --source=path --target=dir-path + + // TODO(florian): add "download" method. + // Must be a tar, tar.gz, tgz, or zip. + // download url/string --path/string="": + + store_ [block] -> none: + if has-stored_: throw "Already saved content for key: $key" + if is-closed_: throw "DirectoryStore is closed" + + // Save files into a temporary directory first, then rename it to the final + // location. + cache_.with-tmp-directory_ key: | tmp-dir | + block.call tmp-dir + key-path := cache_.key-path_ key + key-dir := fs.dirname key-path + directory.mkdir --recursive key-dir + atomic-move-directory_ tmp-dir key-path + + has-stored_ = true + + +atomic-move-file_ source-path/string target-path/string -> none: + // There is a race condition here, but not much we can do about it. + if file.is-file target-path: return + file.rename source-path target-path + +atomic-move-directory_ source-path/string target-path/string -> none: + // There is a race condition here, but not much we can do about it. + if file.is-directory target-path: return + file.rename source-path target-path + +copy-file_ --source/string --target/string -> none: + // TODO(florian): we want to keep the permissions of the original file, + // except that we want to make the file read-only. + in := file.Stream.for-read source + out := file.Stream.for-write target + w := writer.Writer out + w.write-from in + in.close + out.close diff --git a/src/utils_.toit b/src/utils_.toit index 8669504..6b5b308 100644 --- a/src/utils_.toit +++ b/src/utils_.toit @@ -2,6 +2,12 @@ // Use of this source code is governed by an MIT-style license that can be // found in the package's LICENSE file. +import host.directory +import host.file +import host.os +import host.pipe +import system + /** Converts snake-case strings to kebab case. @@ -9,3 +15,54 @@ For example, "hello_world" becomes "hello-world". */ to-kebab str/string -> string: return str.replace --all "_" "-" + +with-tmp-directory [block]: + tmpdir := directory.mkdtemp "/tmp/artemis-" + try: + block.call tmpdir + finally: + directory.rmdir --recursive tmpdir + +tool-path_ tool/string -> string: + if system.platform != system.PLATFORM-WINDOWS: return tool + // On Windows, we use the .exe that comes with Git for Windows. + + // TODO(florian): depending on environment variables is brittle. + // We should use `SearchPath` (to find `git.exe` in the PATH), or + // 'SHGetSpecialFolderPath' (to find the default 'Program Files' folder). + program-files-path := os.env.get "ProgramFiles" + if not program-files-path: + // This is brittle, as Windows localizes the name of the folder. + program-files-path = "C:/Program Files" + result := "$program-files-path/Git/usr/bin/$(tool).exe" + if not file.is-file result: + throw "Could not find $result. Please install Git for Windows" + return result + +/** +Copies the $source directory into the $target directory. + +If the $target directory does not exist, it is created. +*/ +// TODO(florian): this should not use 'tar'. Once we have support for +// symlinks this function should be rewritten. +copy-directory --source/string --target/string: + directory.mkdir --recursive target + with-tmp-directory: | tmp-dir | + // We are using `tar` so we keep the permissions and symlinks. + tar := tool-path_ "tar" + + tmp-tar := "$tmp-dir/tmp.tar" + extra-args := [] + if system.platform == system.PLATFORM-WINDOWS: + // Tar can't handle backslashes as separators. + source = source.replace --all "\\" "/" + target = target.replace --all "\\" "/" + tmp-tar = tmp-tar.replace --all "\\" "/" + extra-args = ["--force-local"] + + // We are using an intermediate file. + // Using pipes was too slow on Windows. + // See https://github.com/toitlang/toit/issues/1568. + pipe.backticks [tar, "c", "-f", tmp-tar, "-C", source, "."] + extra-args + pipe.backticks [tar, "x", "-f", tmp-tar, "-C", target] + extra-args diff --git a/tests/cache_test.toit b/tests/cache_test.toit new file mode 100644 index 0000000..53f710e --- /dev/null +++ b/tests/cache_test.toit @@ -0,0 +1,278 @@ +// Copyright (C) 2023 Toitware ApS. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/LICENSE file. + +import cli.cache +import host.directory +import host.file +import monitor +import expect show * +import writer + +main: + test-file-cache + test-dir-cache + +test-file-cache: + cache-dir := directory.mkdtemp "/tmp/cache_test-" + try: + c := cache.Cache --app-name="test" --path=cache-dir + + key := "key" + expect-not (c.contains key) + value := c.get key: | store/cache.FileStore | + store.save #[1, 2, 3] + expect-equals value #[1, 2, 3] + + value = c.get key: | store | + throw "Should not be called" + expect-equals value #[1, 2, 3] + + path := c.get-file-path key: | store | + throw "Should not be called" + expect-equals path "$cache-dir/key" + content := file.read-content path + expect-equals content #[1, 2, 3] + + key2 := "dir/nested/many/levels/key2" + expect-not (c.contains key2) + value2 := c.get key2: | store/cache.FileStore | + store.save #[4, 5, 6] + expect-equals value2 #[4, 5, 6] + + value2 = c.get key2: | store | + throw "Should not be called" + expect-equals value2 #[4, 5, 6] + + path2 := c.get-file-path key2: | store | + throw "Should not be called" + expect-equals path2 "$cache-dir/$key2" + content2 := file.read-content path2 + expect-equals content2 #[4, 5, 6] + + // Make sure we didn't leave any temporary directories behind. + dir-streamer := directory.DirectoryStream cache-dir + dir-entries := {} + while entry := dir-streamer.next: + dir-entries.add entry + + key3 := "dir/key3" + expect-not (c.contains key3) + value3 := c.get key3: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[7, 8, 9] + store.move tmp-file + expect-equals value3 #[7, 8, 9] + + value3 = c.get key3: | store | + throw "Should not be called" + expect-equals value3 #[7, 8, 9] + + key4 := "dir/key4" + expect-not (c.contains key4) + value4 := c.get key4: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[10, 11, 12] + store.copy tmp-file + expect-equals value4 #[10, 11, 12] + + value4 = c.get key4: | store | + throw "Should not be called" + expect-equals value4 #[10, 11, 12] + + // Note that key5 will not get any value, and there should not be + // any directory for it. + key5 := "dir2/key5" + expect-not (c.contains key5) + expect-throw "fail": c.get key5: | store/cache.FileStore | + throw "fail" + expect-not (c.contains key5) + + // If a move/copy fails, the key doesn't get a value. + exception := catch: c.get key5: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + // Doesn't exist. + store.move tmp-file + expect-not-null exception + expect-not (c.contains key5) + + exception = catch: c.get key5: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + // Doesn't exist. + store.copy tmp-file + expect-not-null exception + expect-not (c.contains key5) + + // However, as soon as a `store` is successful, the first value + // sticks. + key6 := "dir/key6" + expect-not (c.contains key6) + exception = catch: c.get key6: | store/cache.FileStore | + store.save #[13, 14, 15] + store.save #[16, 17, 18] + expect (exception.starts-with "Already saved") + value6 := c.get key6: | store/cache.FileStore | + throw "Should not be called" + expect-equals value6 #[13, 14, 15] + + // Test concurrent cache access. + + // Incremented, when the task is allowed to save the cache value. + semaphore1 := monitor.Semaphore + // Incremented, when the task has finished writing the cache value. + semaphore2 := monitor.Semaphore + + key7 := "dir/key7" + task:: + semaphore1.down + c.get key7: | store/cache.FileStore | + store.save #[19, 20, 21] + semaphore2.up + + value7 := c.get key7: | store/cache.FileStore | + semaphore1.up + semaphore2.down + store.save #[22, 23, 24] + + // The first task wins. + expect-equals value7 #[19, 20, 21] + + // Test concurrent access with copy. + key9 := "dir/key9" + task:: + semaphore1.down + c.get key9: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[31, 32, 33] + store.copy tmp-file + semaphore2.up + + value9 := c.get key9: | store/cache.FileStore | + semaphore1.up + semaphore2.down + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[34, 35, 36] + store.copy tmp-file + + // The first task wins. + expect-equals value9 #[31, 32, 33] + + // Test concurrent access with move. + key11 := "dir/key11" + task:: + semaphore1.down + c.get key11: | store/cache.FileStore | + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[43, 44, 45] + store.move tmp-file + semaphore2.up + + value11 := c.get key11: | store/cache.FileStore | + semaphore1.up + semaphore2.down + store.with-tmp-directory: | tmp-dir | + tmp-file := "$tmp-dir/file" + write-content --path=tmp-file --content=#[46, 47, 48] + store.move tmp-file + + // The first task wins. + expect-equals value11 #[43, 44, 45] + + expect-equals 2 dir-entries.size + expect (dir-entries.contains "key") + expect (dir-entries.contains "dir") + + finally: + directory.rmdir --recursive cache-dir + + +write-content --path/string --content/ByteArray: + stream := file.Stream.for-write path + w := writer.Writer stream + w.write content + w.close + +test-dir-cache: + cache-dir := directory.mkdtemp "/tmp/cache_test-" + try: + c := cache.Cache --app-name="test" --path=cache-dir + + // Test 'move' of the tmp directory. + key := "key" + expect-not (c.contains key) + value := c.get-directory-path key: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + store.move dir + expect-equals value "$cache-dir/$key" + + value = c.get-directory-path key: | store | + throw "Should not be called" + expect-equals value "$cache-dir/$key" + + // Test 'copy' of the tmp directory. + key2 := "key2" + expect-not (c.contains key2) + value2 := c.get-directory-path key2: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[1, 2, 3] + store.move dir + expect-equals value2 "$cache-dir/$key2" + expect-equals #[1, 2, 3] (file.read-content "$value2/file") + + // Test nested directories. + key3 := "dir/key3" + expect-not (c.contains key3) + value3 := c.get-directory-path key3: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[4, 5, 6] + store.move dir + expect-equals value3 "$cache-dir/$key3" + expect-equals #[4, 5, 6] (file.read-content "$value3/file") + + // Test concurrent accesses to the cache. + + // Incremented, when the task is allowed to save the cache value. + semaphore1 := monitor.Semaphore + // Incremented, when the task has finished writing the cache value. + semaphore2 := monitor.Semaphore + + key4 := "dir/key4" + task:: + semaphore1.down + c.get-directory-path key4: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[7, 8, 9] + store.move dir + semaphore2.up + + value4 := c.get-directory-path key4: | store/cache.DirectoryStore | + semaphore1.up + semaphore2.down + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[10, 11, 12] + store.move dir + + expect-equals value4 "$cache-dir/$key4" + // The first task wins. + expect-equals #[7, 8, 9] (file.read-content "$value4/file") + + // Make sure we didn't leave any temporary directories behind. + dir-streamer := directory.DirectoryStream cache-dir + dir-entries := {} + while entry := dir-streamer.next: + dir-entries.add entry + + expect-equals 3 dir-entries.size + expect (dir-entries.contains "key") + expect (dir-entries.contains "key2") + expect (dir-entries.contains "dir") + + finally: + directory.rmdir --recursive cache-dir diff --git a/tests/package.lock b/tests/package.lock index da751d0..aae3f28 100644 --- a/tests/package.lock +++ b/tests/package.lock @@ -1,5 +1,22 @@ +sdk: ^2.0.0-alpha.120 prefixes: cli: .. + host: pkg-host packages: ..: path: .. + prefixes: + fs: pkg-fs + host: pkg-host + pkg-fs: + url: github.com/toitlang/pkg-fs + name: fs + version: 1.0.0 + hash: c816c85022a155f37a4396455da9b27595050de1 + prefixes: + host: pkg-host + pkg-host: + url: github.com/toitlang/pkg-host + name: host + version: 1.11.0 + hash: 7e7df6ac70d98a02f232185add81a06cec0d77e8 diff --git a/tests/package.yaml b/tests/package.yaml index 7cdf3a9..084c550 100644 --- a/tests/package.yaml +++ b/tests/package.yaml @@ -1,3 +1,6 @@ dependencies: cli: path: .. + host: + url: github.com/toitlang/pkg-host + version: ^1.11.0 From 6d1271567512686b10e30aa86eae066a81492a82 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:39:48 +0100 Subject: [PATCH 8/9] Feedback. --- src/cache.toit | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cache.toit b/src/cache.toit index 42c7bc0..49b170b 100644 --- a/src/cache.toit +++ b/src/cache.toit @@ -46,10 +46,9 @@ class Cache: */ constructor --app-name/string: app-name-upper := app-name.to-ascii-upper - env := os.env - if env.contains "$(app-name-upper)_CACHE_DIR": - path := env["$(app-name-upper)_CACHE_DIR"] - return Cache --app-name=app-name --path=path + env-path := os.env.get "$(app-name-upper)_CACHE_DIR" + if env-path: + return Cache --app-name=app-name --path=env-path cache-home := xdg.cache-home return Cache --app-name=app-name --path="$cache-home/$(app-name)" From d4dadbcd01f8cf42a004c34817d61b3602619870 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 17 Nov 2023 15:41:42 +0100 Subject: [PATCH 9/9] Allow to override the cache dir. (#37) --- src/cache.toit | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cache.toit b/src/cache.toit index 874645d..49b170b 100644 --- a/src/cache.toit +++ b/src/cache.toit @@ -37,12 +37,19 @@ class Cache: /** Creates a new cache. - If the \$XDG_CACHE_HOME environment variable is set, the cache is located - at \$XDG_CACHE_HOME/$app-name. Otherwise, the cache will is stored - in \$(HOME)/.cache/$app-name. + Determines the cache directory in the following order: + - If APP_CACHE_DIR (where "APP" is the uppercased version of $app-name) is set, + uses it as the path to the cache. + - If the \$XDG_CACHE_HOME environment variable is set, the cache is located + at \$XDG_CACHE_HOME/$app-name. + - Otherwise, the cache directory is set to \$(HOME)/.cache/$app-name. */ constructor --app-name/string: app-name-upper := app-name.to-ascii-upper + env-path := os.env.get "$(app-name-upper)_CACHE_DIR" + if env-path: + return Cache --app-name=app-name --path=env-path + cache-home := xdg.cache-home return Cache --app-name=app-name --path="$cache-home/$(app-name)"