Allow multiline shebangs in scripts

[vkstrm]

Fix for issue #2024, allowing multiline shebangs, most importantly for use with Nix scripts, I don't know if they are relevant anywhere else.

Manual testing with Nix works for me, and I think error line numbers are retained correctly. The test added is a bit fake (just reads it like a shebang and shell like a comment), but a realistic integration test would require adding Nix to the dependencies which maybe isn't a good idea.

First time contributing, thank you for any feedback!

[casey]

Thanks for the PR! It looks like this code will trigger no matter where in the recipe body the shebang line is. Instead, it should only trigger for the first contiguous shebang lines, and then treat shebang lines after the first non-shebang line normally.

[vkstrm]

f20f493 A suggestion how to do that. Looks a bit more messy, but won't look at shebangs after the ones following the first line. The middle iteration is annoying since it still needs to handle a non-shebang line so that it is added correctly. The last one is cleaner though.

[laniakea64]

Looks a bit more messy,

FWIW there is also #2073 which started to attempt a different approach to implement multi-line shebang functionality. If avoiding messy-looking code is important, might that other PR suggest some way to improve this?

[vkstrm]

Looks a bit more messy,

FWIW there is also #2073 which started to attempt a different approach to implement multi-line shebang functionality. If avoiding messy-looking code is important, might that other PR suggest some way to improve this?

I did look at that first but gave it a go myself since it seems it wasn't being worked on. I guess it depends on how you look at it if that solutions is more appropriate, if the Shebang struct is interested in more than just the actual real shebang? I can see how collecting them there and then using them in script() could be done. If you think that is better I can try.

[laniakea64]

I'm not just dev so not sure what's better. If I were making this patch less messy-looking I'd make the iterator .peekable() and change your middle iteration to something like

        // Any shebang line that follows the first should also be at the top
        while let Some((line, evaluated)) = iter.by_ref().peek() {
          if !line.is_shebang() {
            break;
          }

          script.push_str(evaluated);
          script.push('\n');
          n += 1;
          iter.next();
        }

This avoids duplicating the line-number-matching code and is more concise.

Tried this locally and it passes cargo test here.

[vkstrm]

Thank you for the suggestion! I tried it with peekable, it helped a bit.
Also moved everything out of the Command arm because it didn't do anything different anymore.
2112a3e

[casey]

LGTM! I simplified it slightly to count the number of shebang lines ahead of time, to avoid needing a peekable iterator, and then just skipping however many shebang lines we processed later.