Skip to content

Allow specifying max length when using shortstrings to create short strings #55

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

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

Allow specifying max length when using shortstrings to create short strings #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c56743d
Add shortstrings test with maxlen
Wynand Sep 10, 2021
84f3d57
Fix shortstrings with ss and maxlen
Wynand Sep 10, 2021
d4368a5
update version to 0.3.12
Wynand Sep 10, 2021
45a3a9e
format tests nicely
Wynand Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "ShortStrings"
uuid = "63221d1c-8677-4ff0-9126-0ff0817b4975"
authors = ["Dai ZJ <[email protected]>", "ScottPJones <[email protected]>", "Lyndon White <[email protected]>"]
version = "0.3.11"
version = "0.3.12"

[deps]
BitIntegers = "c3b6d118-76ef-56ca-8cc7-ebb389d030a1"
Expand Down
2 changes: 1 addition & 1 deletion src/base.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ argument `maxlen` is passed.
If the keyword argument `types` is passed with a list (a tuple or Vector) of Unsigned
types, in order of their size, then one of those types will be used.
"""
ShortString(str::Union{String,SubString{String}}, maxlen = sizeof(str); types=def_types) =
ShortString(str::AbstractString, maxlen = sizeof(str); types=def_types) =
Copy link
Member

Choose a reason for hiding this comment

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

If relaxing it always to AbstractString,
need to also test that Test.GenericString works

get_type(max(maxlen,1), types=types)(str)

"""
Expand Down
28 changes: 28 additions & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,34 @@ end
@test ShortString7(ShortString7("ab")) isa ShortString7

@test_throws ErrorException ShortString3(ShortString7("123456"))

# Test creating shortstrings with maxlen
@test ShortString(
ss127"Be honest, do you actually need a string longer than this. Seriously. C'mon this is pretty long.",
127,
) == "Be honest, do you actually need a string longer than this. Seriously. C'mon this is pretty long."
@test ShortString(
ss63"Basically a fairly long string really", 63
) == "Basically a fairly long string really"
@test ShortString(ss31"A Longer String!!!", 31) == "A Longer String!!!"
Comment on lines +127 to +134
Copy link
Member

@oxinabox oxinabox Sep 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we need these test.
They don't test anything different do they?

We should test things with:

  • shortstring below maxlen
  • shortstring at maxlen
  • shortstring above maxlen

@test ShortString(ss15"Short String!!!", 15) == "Short String!!!"
@test ShortString(ss7"ShrtStr", 7) == "ShrtStr"
@test ShortString(ss3"ss3", 3) == "ss3"
@test ShortString("", 0) == ""

@test_throws ErrorException ShortString(
ss127"Be honest, do you actually need a string longer than this. Seriously. C'mon this is pretty long.",
0,
) == "Be honest, do you actually need a string longer than this. Seriously. C'mon this is pretty long."
@test_throws ErrorException ShortString(
ss63"Basically a fairly long string really", 0
) == "Basically a fairly long string really"
@test_throws ErrorException ShortString(
ss31"A Longer String!!!", 0
) == "A Longer String!!!"
@test_throws ErrorException ShortString(ss15"Short String!!!", 0) == "Short String!!!"
@test_throws ErrorException ShortString(ss7"ShrtStr", 0) == "ShrtStr"
Copy link
Member

Choose a reason for hiding this comment

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

Including the right-hand side == here seems like it is adding nothing?
It makes it unclear what should throw the error.

# @test_throws ErrorException ShortString(ss3"ss3", 0) == "ss3" Why doesn't this throw an error?
end

@testset "promote rule" begin
Expand Down