Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expect separator after null byte during encoding (v0.2) #6

Open
SebbyLaw opened this issue Mar 21, 2021 · 6 comments
Open

Expect separator after null byte during encoding (v0.2) #6

SebbyLaw opened this issue Mar 21, 2021 · 6 comments

Comments

@SebbyLaw
Copy link
Member

SebbyLaw commented Mar 21, 2021

Currently the spec does not have the separator occuring after null bytes encoded to bottom. Below is a list of implementations that must be updated to follow this encoding change. Decoding should not need to be changed, as long as it already followed the spec.

Implementation Encode Compliant Decode Compliant
spec (README)
bottom-as
bottom-bash
bottom-c
bottom-d
bottom-dart
bottom-dotnet
bottom-ex
bottom-go
bottom-go-bindings
bottom-hs
bottom-hy
bottom-java
bottom-lua
bottom-matlab
bottom-php
bottom-py
bottom-py-bindings
bottom-rb
bottom-rs
bottom-swift
bottom-web
bottom-webextension
bottom-wl
bottom-workers
bottom-x86
oneline-bottom-py
power-bottom

Also see: #2, bottom-software-foundation/bottom-rs#8, bottom-software-foundation/bottom-go#1

@nihaals
Copy link
Member

nihaals commented Mar 22, 2021

Maybe this should be changed into a table just so both encode and decode can be validated? Null byte was something I saw was ignored in a lot of implementations so it's possible some have a compliant encode but not a compliant decode.

@nihaals
Copy link
Member

nihaals commented Mar 22, 2021

Here's a partial table:

Implementation Encode compliant Decode compliant
bottom-bash
bottom-c

Source

| Implementation | Encode compliant | Decode compliant |
|---|---|---|
| bottom-bash | <ul><li>- [ ] ​</li></ul> | <ul><li>- [ ] ​</li></ul> |
| bottom-c | <ul><li>- [ ] ​</li></ul> | <ul><li>- [ ] ​</li></ul> |
Note there are zero width spaces

@nihaals
Copy link
Member

nihaals commented Mar 22, 2021

It might also be worth adding the version tested
That also means you can differentiate between implementations that have been verified to be noncompliant and implementations that haven't been checked yet

Without a standard testing suite it's hard to get automated testing on all of them, so it could also be possible that an implementation changes from being complaint to noncompliant, so logging versions could help with being clear about what was tested (although tracking changes after an implementation has become completely compliant is probably out of scope for this issue)

@SebbyLaw
Copy link
Member Author

I've added the table. Decode is assumed not compliant until I review it or someone reviews it for me.

It might also be worth adding the version tested
That also means you can differentiate between implementations that have been verified to be noncompliant and implementations that haven't been checked yet

Without a standard testing suite it's hard to get automated testing on all of them, so it could also be possible that an implementation changes from being complaint to noncompliant, so logging versions could help with being clear about what was tested (although tracking changes after an implementation has become completely compliant is probably out of scope for this issue)

Out of the scope of this issue imo.

diamondburned added a commit to bottom-software-foundation/bottom-bash that referenced this issue Mar 22, 2021
@elldritch
Copy link

elldritch commented Apr 2, 2021

FYI, null byte encoding works fine for bottom-hs, which has always encoded separators after null bytes.

For evidence:

a standard testing suite

You could build one of these pretty easily by adapting the bottom-hs test suite to accept adapters for other CLIs. It uses both the bottom-rs tests as well as randomized property-based testing via QuickCheck.

@SebbyLaw
Copy link
Member Author

SebbyLaw commented Apr 2, 2021

Thanks! I've updated the table to match

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants