charmbracelet / vhs

Your CLI home video recorder 📼
MIT License
15.38k stars 261 forks source link

New ENV command breaks SET command in tape file #503

Open rahji opened 4 months ago

rahji commented 4 months ago

Describe the bug

If you add a line like Env PROMPT '>' in the tape file, it has to be after any Set commands or it will cause them to be ignored when tape is run. (They appear in STDOUT but grayed out, and its clear from the resulting gif that they were ignored).

Setup Please complete the following information along with version numbers, if applicable.

To Reproduce Steps to reproduce the behavior:

  1. Create a tape file containing the following and run with tape:
    Output foo.gif
    Env PROMPT "-"
    Set Height 100
    Type "creates a gif, but ignores 'Set Height'"
  2. Create a second tape file containing the following and run with tape:
    Output bar.gif
    Set Height 100
    Env PROMPT "-"
    Type "The tape command will fail with this file since 100 is not a valid 'Height'"

Source Code See above

Expected behavior Both tape files should work properly. Instead, the first one will create foo.gif but will ignore the Set Height command. The second one will fail since the Set Height line is not being ignored (and contains an invalid value)

Screenshots n/a

Additional context This only occurs on the current build. The ENV command is not available in the latest release. @Delta456's #469 was merged after that.

Delta456 commented 4 months ago

In my PR I removed the functionality of ENV PROMPT prompt and it only allows to define env variables.

rahji commented 4 months ago

~ENV PROMPT prompt works - but only if it comes after the SET lines~

Actually, PROMPT doesn't work.

The bug is still accurate otherwise though, since the following tape file causes the Set commands to be ignored (because Env precedes them):

Output demo.gif

Env A "2"
Set Width 1150
Set Height 700
Set FontSize 18

Type "echo $A"
Enter

Sleep 2s
Delta456 commented 4 months ago

I will investigate this and attempt to fix this.

Delta456 commented 4 months ago

After hours of investigation, this happens for all commands before SET. Perhaps all commands need to be written after setting the configuration of a tape.

maaslalani commented 4 months ago

After hours of investigation, this happens for all commands before SET. Perhaps all commands need to be written after setting the configuration of a tape.

Hey @Delta456, this is a documented behaviour of settings: https://github.com/charmbracelet/vhs#settings

Setting must be administered at the top of the tape file. Any setting (except TypingSpeed) applied after a non-setting or non-output command will be ignored.

Delta456 commented 4 months ago

Ah, I missed that. This needs to be told when using the program IMO. Also, this issue can now be closed.

maaslalani commented 4 months ago

Ah, I missed that. This needs to be told when using the program IMO. Also, this issue can now be closed.

Agreed, if a setting is ignored we should display a warning that this setting should appear at the top of the file!

Delta456 commented 4 months ago

Ah, I missed that. This needs to be told when using the program IMO. Also, this issue can now be closed.

Agreed, if a setting is ignored we should display a warning that this setting should appear at the top of the file!

I will add this warning in my future PRs.

Delta456 commented 1 month ago

This issue can be closed as https://github.com/charmbracelet/vhs/pull/508 is merged