search

kristopolous / TickTick

JSON in your Bash scripts
http://9ol.es/TheEmperorsNewClothes.html
Other
579 stars 55 forks source link

Push and Pop may need quotes for the values #4

Closed jn0 closed 12 years ago

jn0 commented 12 years ago

Line #218 in ticktick.sh must read as eval "'$base$nextval'='$value'" and not eval $base$nextval=$value or even the test http://qaa.ath.cx/TheEmperorsNewClothes.html fails.

kristopolous commented 12 years ago

The emperor's new clothes (TENC) code has a quote misuse (an artifact of how push and pop had to be implemented), I'll correct that.

As far as the quotation for the eval, $base$nextval is safe and doesn't need a quote; think about that one, I think it's right here.

But as far as the value goes; the white space is incorrectly collapsed on that value; and if that bug was fixed then they may need quotes; I'm too foggy headed right now to know for sure, but I'll open a bug against me just in case.

  1. The implementation of push and pop need to change to be more easily encapsulated in double quotes, as in the original TENC post (changed as of now). see https://github.com/kristopolous/TickTick/issues/5
  2. The value needs to be escaped and its white space preserved. https://github.com/kristopolous/TickTick/issues/6
  3. The white space for the value need to be quoted. ... this issue has been renamed to refer to this.
kristopolous commented 12 years ago

Fixed as of [1] https://github.com/kristopolous/TickTick/commit/e1ecdb1a81fbd893e05e094ec07ce9713038f87b [2] https://github.com/kristopolous/TickTick/commit/25d4614bb224b871c17a770a5c12e23ca9f28a74

Synopsis: You are correct with the quote issue in the push (see 1)... pop had to be solved a different way (see 2). In order to accommodate things like "key \" tricky stuff", I had to escape the quotes before the assignment; so that's being done too now.

Thanks for filing this. I highly value your input. :)