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.

Sign up for GitHub

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

Jump to bottom

Stringify Bytes #1860

Merged
merged 4 commits into from
Dec 4, 2015
Merged

Stringify Bytes #1860

merged 4 commits into from
Dec 4, 2015

Conversation

lbajolet
Copy link
Contributor

This PR adds some String-like functions to Bytes, along with an abstraction for Byte-based patterns.

Also, a bug was found within Bytes::is_empty and FlatText is now public since there was no real rationale to keep it private

@lbajolet
lib/core/bytes: Fixed broken is_empty
51e3f1c 
Signed-off-by: Lucas Bajolet <r4pass@hotmail.com>
@privat privat added this to the Nit Coding Party #2 - Nitcorn milestone Nov 29, 2015
privat
privat reviewed Nov 29, 2015
View reviewed changes
lib/core/bytes.nit
end

# Trims off the whitespaces at the beginning and the end of `self`
#
Copy link
Member

Choose a reason for hiding this comment

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

can you define whitespace (it can be ambiguous)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"# NOTE: A whitespace is defined here as a byte whose value is <= 0x20" added

@privat
Copy link
Member

privat commented Nov 29, 2015

There is a lot of things (and lot of clones) but I have no better proposal, so +1 but I will wait for other reviewers.

@lbajolet
lib/core: Stringify lib/bytes
4975172 
Signed-off-by: Lucas Bajolet <r4pass@hotmail.com>
@lbajolet
lib/core: Make FlatText public
fea53e1 
Signed-off-by: Lucas Bajolet <r4pass@hotmail.com>
@lbajolet
lib/core: Added join_bytes top-level function to join an array of `…
fd67956 
…Bytes`

Signed-off-by: Lucas Bajolet <r4pass@hotmail.com>
privat added a commit that referenced this pull request Dec 2, 2015
@privat
Merge: Stringify Bytes
e0d0a9b 
This PR adds some String-like functions to Bytes, along with an abstraction for Byte-based patterns.

Also, a bug was found within `Bytes::is_empty` and `FlatText` is now public since there was no real rationale to keep it private

Pull-Request: #1860
Reviewed-by: Jean Privat <jean@pryen.org>
@xymus
Copy link
Contributor

xymus commented Dec 3, 2015

I'm still not convinced that replicating the String API on bytes is a good idea.

My two cents is that there should be a default transformation between Bytes and String so they share an API and are compatible. For example, Bytes could be a Pattern and could implement search(p: Patterrn), using double dispatch we could search bytes within bytes, strings within strings and convert strings to UTF8 bytes in other cases.

But you're the expert and this will be useful so +1.

privat added a commit that referenced this pull request Dec 3, 2015
@privat
Merge: Stringify Bytes
aa72915 
This PR adds some String-like functions to Bytes, along with an abstraction for Byte-based patterns.

Also, a bug was found within `Bytes::is_empty` and `FlatText` is now public since there was no real rationale to keep it private

Pull-Request: #1860
Reviewed-by: Jean Privat <jean@pryen.org>
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
@privat privat merged commit fd67956 into nitlang:master Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lbajolet lbajolet

Labels
None yet
Projects
None yet
Milestone
Nit Coding Party #2 - Nitcorn
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lbajolet @privat @xymus