feat: add fish support

[kxxt]

This PR adds support for https://fishshell.com/ .

https://fishshell.com/docs/current/language.html#quotes

[allenap]

Hi @kxxt, thank you for this!

A few tests are failing. Nothing serious; they only need to be updated a bit.

The implementation of Fish::escape_chars is complicated by is_there_char_after_last_single_quote and true_quotes. I think there's a cleaner way of doing that; I want to give it some thought.

The u8_to_hex change brings nice performance improvements, but it's fiddly to use. It looks like C – which isn't necessarily bad, but I wonder if there's a way to improve its ergonomics. That's not a blocker for this PR though.

[kxxt]

A few tests are failing. Nothing serious; they only need to be updated a bit.

Yes. I find some tests failing as well. It doesn't seem to be caused by this PR.

The implementation of Fish::escape_chars is complicated by is_there_char_after_last_single_quote and true_quotes. I think there's a cleaner way of doing that; I want to give it some thought.

It's complicated because I want to remove unnecessary empty single quotes from output. I didn't figure out a better way to write it at that time. Maybe you can find a better way :)

The u8_to_hex change brings nice performance improvements, but it's fiddly to use. It looks like C – which isn't necessarily bad, but I wonder if there's a way to improve its ergonomics.

I think we will get the performance improvement as long as we avoid the use of the heavy format macro. u8_to_hex is safe rust anyway.

BTW did you consider UTF-8 support? To me I think utf-8 is really necessary so I might try to implement that later. I am quite busy with some other things recently so I will come back at this PR later.

[allenap]

Sorry I left this dormant for a while. However, I've fixed it up now, got the tests passing, and added some more documentation.

One thing was particularly interesting. Do you remember the debug_assert_eq!(sout.pop(), b"\0") calls that were there to defensively check that the code was doing the right thing? When run in release mode they optimised to... nothing. The sout.pop() was not being called.

I hope you're okay with the changes I've made. I really appreciate the work you did you add this; it would not exist without you. I'll make a proper release tomorrow.

[kxxt]

Sorry I left this dormant for a while. However, I've fixed it up now, got the tests passing, and added some more documentation.

Never mind, I forgot this PR as well :)

One thing was particularly interesting. Do you remember the debug_assert_eq!(sout.pop(), b"\0") calls that were there to defensively check that the code was doing the right thing? When run in release mode they optimised to... nothing. The sout.pop() was not being called.

Yes, debug_assert_eq are only meant to be enabled in debug builds.

I hope you're okay with the changes I've made. I really appreciate the work you did you add this; it would not exist without you. I'll make a proper release tomorrow.

Thanks for cleaning the things up!

I think there's still one thing to consider: For fish the quote result might contain newlines. But for other shells the quote result is always a single line. Should we change it to also keep the quote result on a single line? Ideally I think we should provide an argument and let the end-user decide but that might be hard to achieve without changing the interfaces.

[allenap]

I think there's still one thing to consider: For fish the quote result might contain newlines. But for other shells the quote result is always a single line. Should we change it to also keep the quote result on a single line? Ideally I think we should provide an argument and let the end-user decide but that might be hard to achieve without changing the interfaces.

Okay, I think that's an interesting thing to do. Filed as #19.