From 7394441728e998c6e2d71d477615b97ec9e7c776 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Sat, 30 Nov 2019 14:32:24 +0100 Subject: [PATCH 1/5] ;Add shelltests demonstrating bug reported in issue 457 --- tests/addons/addons.test | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/addons/addons.test b/tests/addons/addons.test index 3c291f069..d71054c28 100644 --- a/tests/addons/addons.test +++ b/tests/addons/addons.test @@ -23,3 +23,26 @@ # PATH= ~/.local/bin/stack exec -- hledger #>>>=0 +############################ issue 457 ##################################### +# 5. test for `>` in https://github.com/simonmichael/hledger/issues/457 # +# note a shelltest already provides one level of quoting, so amt:>0 not 'amt:>0' # +# # +$ hledger -f - ui amt:>0 # +>2 !/could not parse/ # +>=1 # + # +# 6. test for `<` in https://github.com/simonmichael/hledger/issues/457 # +$ hledger -f - ui amt:<0 # +>2 !/could not parse/ # +>=1 # + # +# 7. test for `>=` in https://github.com/simonmichael/hledger/issues/457 # +$ hledger -f - ui amt:>=0 # +>2 !/could not parse/ # +>=1 # + # +# 8. test for `<=` in https://github.com/simonmichael/hledger/issues/457 # +$ hledger -f - ui amt:<=0 # +>2 !/could not parse/ # +>=1 # +################################################################################## From 29211868bb56112a7781defaedf2301d4115ca21 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Sat, 30 Nov 2019 14:34:59 +0100 Subject: [PATCH 2/5] Fix issue 457 Issue #457 pointed out that commands such as hledger ui 'amt:>200' failed. This was becasue the process of dispatching from `hledger ui` to `hledger-ui` (note addition of `-`) lost the quotes around `amt:>20` and the `>` character was interpreted as a shell redirection operator, rather than as part of the argument. The machinery for quoting or escaping arguements which cointain characters which require quoting or escaping (thus far whitespace and quotes) already existed. This solution simply adds shell stdio redirection characters to this set. Fixes #457 --- hledger-lib/Hledger/Utils/String.hs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hledger-lib/Hledger/Utils/String.hs b/hledger-lib/Hledger/Utils/String.hs index 03342317d..389c6d2fb 100644 --- a/hledger-lib/Hledger/Utils/String.hs +++ b/hledger-lib/Hledger/Utils/String.hs @@ -111,7 +111,7 @@ underline s = s' ++ replicate (length s) '-' ++ "\n" -- | Double-quote this string if it contains whitespace, single quotes -- or double-quotes, escaping the quotes as needed. quoteIfNeeded :: String -> String -quoteIfNeeded s | any (`elem` s) (quotechars++whitespacechars) = "\"" ++ escapeDoubleQuotes s ++ "\"" +quoteIfNeeded s | any (`elem` s) (quotechars++whitespacechars++redirectchars) = "\"" ++ escapeDoubleQuotes s ++ "\"" | otherwise = s -- | Single-quote this string if it contains whitespace or double-quotes. -- No good for strings containing single quotes. @@ -119,9 +119,10 @@ singleQuoteIfNeeded :: String -> String singleQuoteIfNeeded s | any (`elem` s) whitespacechars = "'"++s++"'" | otherwise = s -quotechars, whitespacechars :: [Char] +quotechars, whitespacechars, redirectchars :: [Char] quotechars = "'\"" whitespacechars = " \t\n\r" +redirectchars = "<>" escapeDoubleQuotes :: String -> String escapeDoubleQuotes = regexReplace "\"" "\"" From 4593963fef657987eb920e3748f60a8cf9c5385f Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Sun, 8 Dec 2019 18:29:13 +0100 Subject: [PATCH 3/5] ;Disable tests according to discussion in #457 https://github.com/simonmichael/hledger/pull/1140#issuecomment-562793255 --- tests/addons/addons.test | 50 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/tests/addons/addons.test b/tests/addons/addons.test index d71054c28..0f0154cfe 100644 --- a/tests/addons/addons.test +++ b/tests/addons/addons.test @@ -23,26 +23,30 @@ # PATH= ~/.local/bin/stack exec -- hledger #>>>=0 -############################ issue 457 ##################################### -# 5. test for `>` in https://github.com/simonmichael/hledger/issues/457 # -# note a shelltest already provides one level of quoting, so amt:>0 not 'amt:>0' # -# # -$ hledger -f - ui amt:>0 # ->2 !/could not parse/ # ->=1 # - # -# 6. test for `<` in https://github.com/simonmichael/hledger/issues/457 # -$ hledger -f - ui amt:<0 # ->2 !/could not parse/ # ->=1 # - # -# 7. test for `>=` in https://github.com/simonmichael/hledger/issues/457 # -$ hledger -f - ui amt:>=0 # ->2 !/could not parse/ # ->=1 # - # -# 8. test for `<=` in https://github.com/simonmichael/hledger/issues/457 # -$ hledger -f - ui amt:<=0 # ->2 !/could not parse/ # ->=1 # -################################################################################## +# ############################ issue 457 ##################################### +# # +# These tests are commented out, as they are not working properly. See: # +# https://github.com/simonmichael/hledger/pull/1140#issuecomment-562793255 # +# # +# # 5. test for `>` in https://github.com/simonmichael/hledger/issues/457 # +# # note a shelltest already provides one level of quoting, so amt:>0 not 'amt:>0' # +# # # +# $ hledger -f - ui amt:>0 # +# >2 !/could not parse/ # +# >=1 # +# # +# # 6. test for `<` in https://github.com/simonmichael/hledger/issues/457 # +# $ hledger -f - ui amt:<0 # +# >2 !/could not parse/ # +# >=1 # +# # +# # 7. test for `>=` in https://github.com/simonmichael/hledger/issues/457 # +# $ hledger -f - ui amt:>=0 # +# >2 !/could not parse/ # +# >=1 # +# # +# # 8. test for `<=` in https://github.com/simonmichael/hledger/issues/457 # +# $ hledger -f - ui amt:<=0 # +# >2 !/could not parse/ # +# >=1 # +# ################################################################################## From c6da152e9a2d42d95bab82f4b690a4c835ba8c13 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Sun, 8 Dec 2019 18:23:48 +0100 Subject: [PATCH 4/5] ;convert extant addon tests to shellrunner format 3 --- tests/addons/addons.test | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/addons/addons.test b/tests/addons/addons.test index 0f0154cfe..271461a86 100644 --- a/tests/addons/addons.test +++ b/tests/addons/addons.test @@ -4,24 +4,24 @@ # they always run the first hledger executable in PATH # 1. flags after an add-command are handled by the add-on - PATH=$PATH:. hledger addon --help ->>> /hledger-addon/ ->>>=0 +$ PATH=$PATH:. hledger addon --help +> /hledger-addon/ +>=0 # 2. add-on flags which are not also defined in the main executable are a problem - PATH=$PATH:. hledger addon --addonflag ->>>2 /Unknown flag: --addonflag/ ->>>=1 +$ PATH=$PATH:. hledger addon --addonflag +>2 /Unknown flag: --addonflag/ +>=1 # 3. hledger main executable ignores anything after -- (and hides the -- from the add-on) - PATH=$PATH:. hledger addon --help -- --addonflag ->>> /hledger-addon/ ->>>=0 +$ PATH=$PATH:. hledger addon --help -- --addonflag +> /hledger-addon/ +>=0 # TODO how to reliably ensure no addons but still find the hledger executable ? # 4. having no addons shouldn't break the commands list -# PATH= ~/.local/bin/stack exec -- hledger -#>>>=0 +# $ PATH= ~/.local/bin/stack exec -- hledger +# >=0 # ############################ issue 457 ##################################### # # From 24fbe313dd6cda6ff85a2c688cd101748364d86c Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Sat, 30 Nov 2019 19:21:23 +0100 Subject: [PATCH 5/5] ;Fix rewrite tests after #457 fix As a consequence of fixing #457, two rewrite tests fail, because extra quotes appear in the output of the commands being tested. These quotes appear in comments which reflect the command which was run in order to produce the output. Instead of ; generated-posting: = assets:bank and amt:<0 we now get ; generated-posting: = assets:bank and "amt:<0" Given that the quotes around `amt:<0` are necessary for the command to work properly, these quotes should really be there in the expected output. Hence this commit modifies the expected output of the tests, by adding in the quotes. This makes the tests pass once more. --- tests/rewrite.test | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/rewrite.test b/tests/rewrite.test index 93e058fb5..39b698d8c 100644 --- a/tests/rewrite.test +++ b/tests/rewrite.test @@ -142,14 +142,14 @@ hledger rewrite -f- assets:bank and 'amt:<0' --add-posting 'expenses:fee $5' - 2016/01/01 withdraw ; modified: assets:cash $20 assets:bank - expenses:fee $5 ; generated-posting: = assets:bank and amt:<0 - assets:bank $-5 ; generated-posting: = assets:bank and amt:<0 + expenses:fee $5 ; generated-posting: = assets:bank and "amt:<0" + assets:bank $-5 ; generated-posting: = assets:bank and "amt:<0" 2016/01/02 withdraw ; modified: assets:cash assets:bank $-30 - expenses:fee $5 ; generated-posting: = assets:bank and amt:<0 - assets:bank $-5 ; generated-posting: = assets:bank and amt:<0 + expenses:fee $5 ; generated-posting: = assets:bank and "amt:<0" + assets:bank $-5 ; generated-posting: = assets:bank and "amt:<0" >>>2 >>>=0 @@ -240,14 +240,14 @@ hledger rewrite --diff -f- assets:bank and 'amt:<0' --add-posting 'expenses:fee +2016/01/01 withdraw ; modified: assets:cash $20 assets:bank -+ expenses:fee $5 ; generated-posting: = assets:bank and amt:<0 -+ assets:bank $-5 ; generated-posting: = assets:bank and amt:<0 ++ expenses:fee $5 ; generated-posting: = assets:bank and "amt:<0" ++ assets:bank $-5 ; generated-posting: = assets:bank and "amt:<0" @@ -5,3 +7,5 @@ -2016/01/02 withdraw +2016/01/02 withdraw ; modified: assets:cash assets:bank $-30 -+ expenses:fee $5 ; generated-posting: = assets:bank and amt:<0 -+ assets:bank $-5 ; generated-posting: = assets:bank and amt:<0 ++ expenses:fee $5 ; generated-posting: = assets:bank and "amt:<0" ++ assets:bank $-5 ; generated-posting: = assets:bank and "amt:<0" >>>2 >>>=0