search

elves / upgrade-scripts-for-0.17

Upgrade Elvish scripts for 0.17
BSD 2-Clause "Simplified" License
4 stars 2 forks source link

Not a serious issue: running the conversion tool twice #3

Closed krader1961 closed 3 years ago

krader1961 commented 3 years ago

This is just a FYI. I do not actually expect any changes to accommodate this scenario. I accidentally ran this tool twice in a row using the latest version of the tool as I write this:

upgrade-scripts-for-0.17 -lambda -w **[type:regular].elv

That resulted in a sequence of errors like this:

Multiple parse errors:
  should be '}'
    lib/interactive.elv, line 31:
      fn hr {||]
  unexpected rune ']'
    lib/interactive.elv, line 31:
      fn hr {||]

Which is not at all surprising yet still surprised me. It made we ponder whether it is possible to do better by only converting a file once? The obvious approach is to add a prefix, such as "# elvish 0.17.0\n", to the transformed output. The presence of that prefix causes the conversion tool to ignore the file. That, however, is a fragile mechanism with ambiguities. I don't expect this to be resolved but wanted to document the problem in case there was an obvious (other than to me) solution.

krader1961 commented 3 years ago

Also, I ran this against my ~/.config/elvish directory as it existed before my previous invocation of this tool. With the exception of the removal of the code that was so old it had parse errors.

Please close this as soon as you determine there is no reasonable way to obviate problems caused by running the tool twice in a row.

xiaq commented 3 years ago

The tool is supposed to be idempotent; at least it shouldn't turn a valid Elvish program into an invalid one. Can you share more of script after one conversion that becomes invalid after the second run?

xiaq commented 3 years ago

@krader1961 looking at the code you are having problematic it seems to be a result not of running the tool twice, but with an empty argument list? That was raised in #4 and fixed now.

krader1961 commented 3 years ago

@xiaq Yes, that is precisely the problem I ran into. This

set edit:insert:binding[Ctrl-R] = []{ interactive:history >/dev/tty 2>&1 }

was being translated to this

set edit:insert:binding[Ctrl-R] = {||] interactive:history >/dev/tty 2>&1 }

The errors I saw from running the migration tool was apparently due to my elvish and associated source being ~2 months old.