Skip to content
New issue

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

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

Already on GitHub? # to your account

*: switch everything to govis #121

Merged
merged 16 commits into from
Feb 15, 2017
Merged

*: switch everything to govis #121

merged 16 commits into from
Feb 15, 2017

Conversation

cyphar
Copy link
Contributor

@cyphar cyphar commented Feb 15, 2017

Now that we have govis, move everything to using govis.{Vis,Unvis} and
then remove the cvis build tags (because that code no longer exists).

Fixes #120
Signed-off-by: Aleksa Sarai asarai@suse.de

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Also add some unit tests -- one of which currently fails due to ongoing
design discussion about how certain escape codes should be handled.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
This is a stopgap while I figure out how I should go about implementing
vis(3). It's also important to have some vis(3) implementation so I can
do integration tests on round-trips.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
In particular, previously such escape handling would break because we
would attempt to encode characters >0x7f as runes -- which would then
result in escapes that want to encode multi-byte characters breaking.

There's still some work necessary in Vis() to make it act sanely when it
comes to arbitrary bit streams. Not to mention that we need to figure
out what we actually want to do there...

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
While these characters are really weird to handle, here is a fairly
simple implementation that need some more testing (and a proper
secondary source to compare against).

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
This results in all multi-byte characters to be encoded in a way that
naive unvis(3) implementations will not bork up the encoding. In
addition, it also ensures that the output of Vis will always be ASCII
*only*.

Also test far more cases in *_test.go when it comes to different flags,
and do far more tests to ensure that the output of Vis() makes sense.
These outputs come directly from vis(3) and so are useful regression
tests to ensure that the handling of Vis() is identical to the original.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
This is necessary to ensure that strings I don't have examples of work
properly. The same thing applies for double-encode and double-decode
usecases.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
govis is a reimplementation of vis(3) and unvis(3) specifically made to
be unicode aware. It was specifically rewritten to replace cvis and the
other go vis reimplementation we have in go-mtree.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
Now that we have govis, move everything to using govis.{Vis,Unvis} and
then remove the cvis build tags (because that code no longer exists).

Signed-off-by: Aleksa Sarai <asarai@suse.de>
@vbatts
Copy link
Owner

vbatts commented Feb 15, 2017

LGTM

@vbatts vbatts merged commit 7b6f89f into vbatts:master Feb 15, 2017
@cyphar
Copy link
Contributor Author

cyphar commented Feb 15, 2017

❤️

@cyphar cyphar deleted the switch-to-govis branch February 15, 2017 18:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants