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

Fix unicode.split with seperators examples #17176

Merged
merged 9 commits into from
Feb 26, 2021
Merged

Conversation

zetashift
Copy link
Contributor

Fixes: #17174
https://nim-lang.org/docs/unicode.html#split.i%2Cstring%2CRune%2Cint didn't work with the set constructor {} so replaced it with a string that's converted to an openArray compatible type containing Runes.

If there is a better/more idiomatic way to get Runes from a set/string, then please do tell, first time interacting with Unicode module.

https://nim-lang.org/docs/unicode.html#split.i%2Cstring%2CRune%2Cint didn't work with the set constructor `{}` so replaced it with a string that's converted to an openArray compatible type containing `Rune`s.
lib/pure/unicode.nim Outdated Show resolved Hide resolved
@zetashift
Copy link
Contributor Author

zetashift commented Feb 25, 2021

@timotheecour using sugar.cycle is hard(or I am using it wrong) because toRunes returns a seq[Rune] and the collect macro packs that into a seq[seq[Rune]] should I just use a string as a separators list?

import sugar

let splittedBySequence = newSeq[string]()
let separators = collect(newSeq):
  for separator in @[";", ":", "$"]:
    separator.toRunes

for word in split("this:is;an$example", separators):
  splittedBySequence.add(word)

doAssert splittedBySequence = @["this", "is", "an", "example"]

@zetashift
Copy link
Contributor Author

I currently have this, is this a decent example?

    # And the following code splits a string using a sequence of Runes.
    import sugar

    var splittedBySequence = newSeq[string]()
    let separators = collect(newSeq):
      for separator in ";:$".runes:
        separator

    for word in split("this:is;an$example", separators):
      splittedBySequence.add(word)

    doAssert splittedBySequence == @["this", "is", "an", "example"]

@ringabout
Copy link
Member

ringabout commented Feb 25, 2021

Looks good!

nit:

import std/sugar


var splittedBySequence = newSeq[string]()
let separators = collect:
  for separator in ";:$".runes:
    separator

for word in split("this:is;an$example", separators):
  splittedBySequence.add(word)

# use assert 
assert splittedBySequence == @["this", "is", "an", "example"]

Or even one line

let separators = collect(for separator in ";:$".runes: separator)

@zetashift
Copy link
Contributor Author

I didn't know you could use collect like that, the examples didn't mention it! :D, awesome thanks.

lib/pure/unicode.nim Outdated Show resolved Hide resolved
lib/pure/unicode.nim Outdated Show resolved Hide resolved
@zetashift
Copy link
Contributor Author

Last question is it better to group the imports?

    import std/sequtils

    assert toSeq("hÃllo\lthis\lis an\texample\l".split) ==
      @["hÃllo", "this", "is", "an", "example", ""]

    # And the following code splits the same string using a sequence of Runes.
    import std/sugar

    var splittedBySequence = newSeq[string]()
    let separators = collect(for separator in ";:$".runes: separator)

Turn the imports into a top level:
import std/[sequtils, sugar]
Not sure which makes it better :(

lib/pure/unicode.nim Outdated Show resolved Hide resolved
@timotheecour
Copy link
Member

Last question is it better to group the imports?

yes but turns out you don't even need collect here, in all your examples, toSeq is enough.
(Note: nim-lang/RFCs#76 would allow the more explicit: from std/[sugar, sequtils] import toSeq, collect but again, that's not needed here)

@timotheecour
Copy link
Member

LGTM, thanks!
further cleanup PR's welcome (see also https://github.com/nim-lang/Nim/pulls?q=is%3Apr+author%3Akonsumlamm+sort%3Aupdated-desc for similar ones, in particular changing code-block (often broken since not run in CI, more verbose, etc) to runnableExamples is an easy way to get started, starting with the most commonly used modules that still rely on code-block

@zetashift
Copy link
Contributor Author

I would love to do more of these! I am gonna ask a lot of questions though since I'm not familiar with all of the stdlib modules, if that's okay haha!

@Araq Araq merged commit c7d6e4c into nim-lang:devel Feb 26, 2021
@zetashift zetashift deleted the patch-1 branch February 26, 2021 16:57
ringabout pushed a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
* Fix unicode.split with seperators examples

https://nim-lang.org/docs/unicode.html#split.i%2Cstring%2CRune%2Cint didn't work with the set constructor `{}` so replaced it with a string that's converted to an openArray compatible type containing `Rune`s.

* Add runnableExamples to unicode.split

* Add runnableExamples to split with single separator too

* Simplify runnableExamples unicode.split

* Improve the rest of the runnableExamples with the simplified code

* Simplify runnableExamples of unicode.split even more

* Formatted unicode.split example

* Update lib/pure/unicode.nim

Co-authored-by: zetashift <rishi2@laptop.localdomain>
Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
* Fix unicode.split with seperators examples

https://nim-lang.org/docs/unicode.html#split.i%2Cstring%2CRune%2Cint didn't work with the set constructor `{}` so replaced it with a string that's converted to an openArray compatible type containing `Rune`s.

* Add runnableExamples to unicode.split

* Add runnableExamples to split with single separator too

* Simplify runnableExamples unicode.split

* Improve the rest of the runnableExamples with the simplified code

* Simplify runnableExamples of unicode.split even more

* Formatted unicode.split example

* Update lib/pure/unicode.nim

Co-authored-by: zetashift <rishi2@laptop.localdomain>
Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>
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.

split example from std/unicode doesn't run
4 participants