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

To Python bytes #370

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

To Python bytes #370

wants to merge 10 commits into from

Conversation

edhinard
Copy link

Hi,
Here is an operation that missed me. With "To Python bytes" I can directly copy the output in a script.
As stated in the description: the UTF-8 encoded string ça ma couté 20€ becomes b'\xc3\xa7a ma cout\xc3\xa9 20\xe2\x82\xac'

@CLAassistant
Copy link

CLAassistant commented Sep 24, 2018

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ edhinard
❌ harry1234eer
You have signed the CLA already but the status is still pending? Let us recheck it.

@edhinard
Copy link
Author

I think this operation can be generalized for other programming languages. It could be named "To bytes literal" and have a parameter for choosing language among: C, Javascript, Go, Python...
I will update my proposition in this direction.
By the way, how do I write unit tests ?

@n1474335
Copy link
Member

Thanks for the contribution. I agree that this could be generalised. Perhaps "To Byte String Literal" would be suitable? You could support a number of common languages. It would also be good to have the opposite operation included as well: "From Byte String Literal".

@edhinard
Copy link
Author

edhinard commented Oct 2, 2018

Here is my contribution. A module to convert any byte sequence To Byte String Literal in C, Go and Python. An example derived from the test cases:

  • input (UTF-8) = Il m'a dit : "ça ma couté 20€".

  • C output = "Il m'a dit : \"\xc3\xa7""a ma cout\xc3\xa9 20\xe2\x82\xac\"."

  • Go output = ([]byte)("Il m'a dit : \"\xc3\xa7a ma cout\xc3\xa9 20\xe2\x82\xac\".")

  • Python output = b'Il m\'a dit : "\xc3\xa7a ma cout\xc3\xa9 20\xe2\x82\xac".'

I will be pleased to add more languages provided I got an idea how literals look like. But I will not propose the opposite operation as I would be afraid to implement a spurious algorithm and to forgot corner cases.

@brotherdust
Copy link

Hi All!
This would be very useful to have. What's the ETA for getting it merged?
Thanks!

@d98762625
Copy link
Member

@edhinard @brotherdust The Travis builds are failing on this PR, seemingly due to an incorrect import in the tests index file. Feel free to contribute to this PR to help get it merged!

Copy link
Member

@d98762625 d98762625 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great PR. I've added some comments for things like formatting and a suggestion on how you can get the build working 👍

@d98762625
Copy link
Member

Sorry, I didn't inspect the files closely enough. It seems you've placed your test file in the wrong place.

test/tests/operations/ByteStringLiteral.mjs

should be in

tests/operations/tests/ByteStringLiteral.mjs

Then you will need to adjust the import path as I mentioned in the in-file comment.

You can run the tests locally to make sure they pass by running npm install and then npm test

@edhinard
Copy link
Author

edhinard commented Sep 9, 2019

it looks like tests are passing

@d98762625
Copy link
Member

Great! Before we can merge this, we need you to sign the Contributor License Agreement, which you can do here.

You don't have to sign this, but we will not be able to use your software without it.

@edhinard
Copy link
Author

edhinard commented Sep 9, 2019

I signed the CLA last year already. When I follow the link it is said "You have signed the CLA for gchq/CyberChef". But the license/cla task is still pending

@n1474335
Copy link
Member

n1474335 commented Sep 9, 2019

According to the CLA Assistant comment, @harry1234eer needs to sign it as they have made some commits to this PR.

@Algoinde
Copy link

@harry1234eer are you willing to sign the CLA? It would be cool to have this merged, I think it's the only thing that is blocking.

@a3957273
Copy link
Member

This has been a long time without the CLA being signed. Would anyone be willing to rewrite @harry1234eer's parts of this PR?

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

Successfully merging this pull request may close these issues.

8 participants