smoosh icon indicating copy to clipboard operation
smoosh copied to clipboard

Word splitting: filter out empty fields

Open tucak opened this issue 4 years ago • 9 comments

I'm unsure where the POSIX specification mentions this, but all of the implementations I tried (bash, dash, mksh, zsh) seem to agree on it.

tucak avatar Nov 20 '19 20:11 tucak

Interesting, thank you! What did Smoosh do on this test case before your changes?

mgree avatar Nov 21 '19 19:11 mgree

Before the change, some of it would end up as an empty field, but the last one was ignored. The output of the test was:

Default:
3
3
4
4
Colon:
3
4
4
4
Unset:
3
3
4
4

tucak avatar Nov 21 '19 20:11 tucak

Gotcha. Let me double check that this change doesn't hurt POSIX conformance. (If I don't get back to you within a week, please ping me!)

mgree avatar Nov 21 '19 21:11 mgree

I wasn't satisfied with the previous approach as it didn't actually fix the actual issue that caused the empty fields to remain. They should have been removed in combine_fields as they only contained white-space. The commit I added should fix it, by turning the user-field separators following the expanded words into whitespace.

The additional test passes on bash, dash, mksh and zsh (I did not test any other), but fails on smoosh without this commit. In case you are curious, the results without this change (the actual argument counts) are:

  • 1 (correct)
  • 3 (incorrect)
  • 2 (correct)
  • 5 (incorrect)
  • 3 (correct)

tucak avatar Dec 02 '19 21:12 tucak

Thanks for the update! I still haven't had a chance to fix the POSIX suite under Docker, but will hopefully have time after I'm back from Thanksgiving travel.

mgree avatar Dec 02 '19 22:12 mgree

Another change, this time to preserve delimiters for the last variable in read. Before this, it would replace every delimiter with a space.

It's a bit more involved than the previous changes, but I found no easier way to do it.

tucak avatar Dec 22 '19 21:12 tucak

I'm planning on migrating to GPLv3 for Smoosh. (Integrating with the GPLv3 Morbig parser will require it.)

Would you like me to merge your code first, while things are still under an MIT license, or is it okay if I wait and the merged code will be under GPLv3?

As I make the change, I plan on making a CONTRIBUTORS.md file in the root, and I'd like to acknowledge your work there. How would you like to be listed?

mgree avatar Aug 26 '20 17:08 mgree

Take your time, I don't mind the GPL at all. Do you need my legal name for the contributors file?

tucak avatar Aug 27 '20 18:08 tucak

I'm honestly unsure what I need. I think the licensing itself works best if contributors disclaim copyright/assign it to me. I was thinking of CONTRIBUTORS.md as being for credit more than for any licensing issues, but I'm really not that up on the details.

If you have a thought or preference here, I'm happy to honor it.

mgree avatar Aug 27 '20 18:08 mgree