# Rstrip minor bug

**URL:** <https://discourse.julialang.org/t/rstrip-minor-bug/8263>\
**Category:** Internals & Design\
**Created:** [January 10, 2018, 8:04am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263 "2018-01-10T08:04:23Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![tk3369](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tk3369/32/2824_2.png) [@tk3369](https://discourse.julialang.org/u/tk3369)\
**Post date:** [January 10, 2018, 8:04am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/1 "2018-01-10T08:04:23Z")

</div>

This code seems wrong though should be fine under normal circumstances… `SubString(s, 1:i)` should have been `SubString(s, a:i)`?

From [base/strings/util.jl](https://github.com/JuliaLang/julia/blob/master/base/strings/util.jl)

```julia
function rstrip(s::AbstractString, chars::Chars=_default_delims)
    a = start(s)
    i = endof(s)
    while a ≤ i
        c = s[i]
        j = prevind(s, i)
        c in chars || return SubString(s, 1:i)
        i = j
    end
    SubString(s, a, a-1)
end

```

---

<div class="post-metadata">

**Author:** ![nalimilan](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/nalimilan/32/147_2.png) [@nalimilan](https://discourse.julialang.org/u/nalimilan)\
**Post date:** [January 10, 2018, 10:00am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/2 "2018-01-10T10:00:14Z")

</div>

I think strings are now expected to use `1` as their first index, so that should always work. But it’s indeed a bit silly to call `start` in one place and use `1` in another. Would you make a pull request to use `a`? You can edit the file directly on GitHub.

---

<div class="post-metadata">

**Author:** ![yuyichao](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/yuyichao/32/20_2.png) [@yuyichao](https://discourse.julialang.org/u/yuyichao)\
**Post date:** [January 10, 2018, 10:33am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/3 "2018-01-10T10:33:26Z")

</div>

> [@nalimilan](#):
>
> But it’s indeed a bit silly to call start in one place and use 1 in another.

That’s wrong in any case. `start` does **NOT** give you the first index.

> [@nalimilan](#):
>
> Would you make a pull request to use a?

So the correct fix is to just replace a with 1.

---

<div class="post-metadata">

**Author:** ![yuyichao](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/yuyichao/32/20_2.png) [@yuyichao](https://discourse.julialang.org/u/yuyichao)\
**Post date:** [January 10, 2018, 10:40am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/4 "2018-01-10T10:40:48Z")

</div>

Also ref [https://github.com/JuliaLang/julia/pull/25458](https://github.com/JuliaLang/julia/pull/25458) for the real way to get non-1 first index.

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 10, 2018, 1:06pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/5 "2018-01-10T13:06:27Z")

</div>

yuyichao is correct here: it is the last line that should be fixed, to be either `Substring(s, 1:0)` or simply `""`.  
I think the second might be preferable, because you don’t want to keep around a unnecessary pointer to s, that might prevent it from getting GCed laster.

This is a common confusion, that the iterator state == index, which isn’t always true.

---

<div class="post-metadata">

**Author:** ![nalimilan](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/nalimilan/32/147_2.png) [@nalimilan](https://discourse.julialang.org/u/nalimilan)\
**Post date:** [January 10, 2018, 1:21pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/6 "2018-01-10T13:21:39Z")

</div>

Yeah, that’s right. This is what happens when you try to write generic code without any actual implementations for which indices and iteration state differ. 🙂 Since strings are now required to use `1` as their first index, we can as well use `1` everywhere instead of `a`.

However, we can’t return `""` because of type stability.

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 10, 2018, 1:29pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/7 "2018-01-10T13:29:48Z")

</div>

True - for my `Strs.jl` code, I have an `empty_str(::Type{S}) where {S<:...} = ...` for that case, so that all “” values are the same (which is fine since these strings are immutable).  
Possibly `S("")` would work, if you have `function rstrip(s::S, chars::Chars=_default_delims) where {S<:AbstractString}`

---

<div class="post-metadata">

**Author:** ![stevengj](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/stevengj/32/71_2.png) [@stevengj](https://discourse.julialang.org/u/stevengj)\
**Post date:** [January 10, 2018, 1:40pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/8 "2018-01-10T13:40:57Z")

</div>

> [@ScottPJones](#):
>
> True - for my Strs.jl code, I have an empty\_str(::Type{S}) where {S\<:…} = … for that case, so that all “” values are the same (which is fine since these strings are immutable).
> 
> Possibly S(“”) would work, if you have function rstrip(s::S, chars::Chars=\_default\_delims) where {S\<:AbstractString}

Base already defines `one` for this. (Since `*` is string concatenation, `one` returns the empty string.) But it is not applicable in this case because of type stability: `rstrip` must return a `SubString` in all cases.

---

<div class="post-metadata">

**Author:** ![nalimilan](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/nalimilan/32/147_2.png) [@nalimilan](https://discourse.julialang.org/u/nalimilan)\
**Post date:** [January 10, 2018, 1:46pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/9 "2018-01-10T13:46:41Z")

</div>

We could return `SubString(one(s), 1:0)` though.

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 10, 2018, 1:53pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/10 "2018-01-10T13:53:15Z")

</div>

> [@stevengj](#):
>
> Base already defines one for this. (Since \* string concatenation, one returns the empty string.)

I’d missed that (rather strange IMO) change, to define one for a string.  
I suppose I’ll need to add that to `Strs.jl` for my string types.

---

<div class="post-metadata">

**Author:** ![stevengj](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/stevengj/32/71_2.png) [@stevengj](https://discourse.julialang.org/u/stevengj)\
**Post date:** [January 10, 2018, 1:57pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/11 "2018-01-10T13:57:29Z")

</div>

> [@ScottPJones](#):
>
> I’d missed that (rather strange IMO) change, to define one for a string.

It is an inevitable consequence of using `*` for string concatenation (which has been discussed ad nauseam, so let’s please not re-open that debate here), since `one` is defined as the multiplicative identity. [Add `one` for AbstractString by bdeonovic · Pull Request #19548 · JuliaLang/julia · GitHub](https://github.com/JuliaLang/julia/pull/19548)

(If we used `+` for string concatenation, then `zero` would return the empty string for the same reason.)

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 10, 2018, 2:18pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/12 "2018-01-10T14:18:02Z")

</div>

> [@stevengj](#):
>
> (If we used + for string concatenation, then zero would return the empty string for the same reason.)

I’ve never suggested using `+` for string concatenation, I, like many others, think that `*` is a bad form of punning, and I see that it was said that defining `one` for strings should be taken as an argument _against_ that sort of punning. [Add `one` for AbstractString by bdeonovic · Pull Request #19548 · JuliaLang/julia · GitHub](https://github.com/JuliaLang/julia/pull/19548#issuecomment-266434000) and [Add `one` for AbstractString by bdeonovic · Pull Request #19548 · JuliaLang/julia · GitHub](https://github.com/JuliaLang/julia/pull/19548#issuecomment-266437476).

Also, if this was so inevitable, why did it take 8 years since [RepString for efficient representation of repeated strings. · JuliaLang/julia@b7faaf9 · GitHub](https://github.com/JuliaLang/julia/commit/b7faaf9628e939c6e75365918dfb6130ebf5d9ed) for it to be added?

---

<div class="post-metadata">

**Author:** ![stevengj](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/stevengj/32/71_2.png) [@stevengj](https://discourse.julialang.org/u/stevengj)\
**Post date:** [January 10, 2018, 3:43pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/13 "2018-01-10T15:43:11Z")

</div>

This has all been debated ad nauseam elsewhere. I’m not interested in re-iterating.

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 10, 2018, 4:13pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/14 "2018-01-10T16:13:51Z")

</div>

I just wanted to point out that having to define `one` (or `zero`) for strings was not such an obvious or inevitable consequence of the choice of `*` for concatenation.  
Is it documented, as part of the API that needs to be implemented by anybody adding a new `AbstractString` type, as I am doing?

---

<div class="post-metadata">

**Author:** ![Tamas\_Papp](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tamas_papp/32/25949_2.png) [@Tamas\_Papp](https://discourse.julialang.org/u/Tamas_Papp)\
**Post date:** [January 10, 2018, 4:41pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/15 "2018-01-10T16:41:41Z")

</div>

I guess if you define `convert`, [this fallback](https://github.com/JuliaLang/julia/blob/0abc2631e1d7527d1925207658e567e9d0bc3ae5/base/strings/basic.jl#L219) should take care of it.

---

<div class="post-metadata">

**Author:** ![tk3369](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tk3369/32/2824_2.png) [@tk3369](https://discourse.julialang.org/u/tk3369)\
**Post date:** [January 11, 2018, 5:15am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/16 "2018-01-11T05:15:42Z")

</div>

Opened PR. I hope I did it correctly… first timer 😛

[https://github.com/JuliaLang/julia/pull/25505](https://github.com/JuliaLang/julia/pull/25505)

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 11, 2018, 8:13am UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/17 "2018-01-11T08:13:41Z")

</div>

Nice! I just verified, the fallback works just fine for my strings package - one(UTF32Str) gives me a “” of type UTF32Str, as expected, without me having to had a method for it.

---

<div class="post-metadata">

**Author:** ![NaOH](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/naoh/32/5632_2.png) [@NaOH](https://discourse.julialang.org/u/NaOH)\
**Post date:** [January 14, 2018, 6:50pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/18 "2018-01-14T18:50:23Z")

</div>

Welcome! 😃

I see the PR was merged without a test! 😮 When would the previous version fail?

Could anyone suggest a test case?

---

<div class="post-metadata">

**Author:** ![ScottPJones](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/scottpjones/32/146_2.png) [@ScottPJones](https://discourse.julialang.org/u/ScottPJones)\
**Post date:** [January 14, 2018, 7:09pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/19 "2018-01-14T19:09:28Z")

</div>

I think it would be an issue of a string type where the value of `start(str)` is not `1`.  
You could create a reversed string type, that returned a start value of `sizeof(str)`, for example, and used `prevind` to walk backwards, for example, to show the bug.

---

<div class="post-metadata">

**Author:** ![nalimilan](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/nalimilan/32/147_2.png) [@nalimilan](https://discourse.julialang.org/u/nalimilan)\
**Post date:** [January 14, 2018, 7:13pm UTC](https://discourse.julialang.org/t/rstrip-minor-bug/8263/20 "2018-01-14T19:13:53Z")

</div>

I guess we could change the `GenericString` type, which is used to test that the string functions do not make more assumptions than the interface allows, to have `start` return something different from `1`.

[Next page](https://discourse.julialang.org/t/rstrip-minor-bug/8263.md?page=2)