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

feat: add words function to standard library #75

Merged

Ph0enixKM merged 9 commits into amber-lang:master from kilianc:patch-1

May 23, 2024

Contributor

kilianc commented May 22, 2024

working on tests

kilianc added 2 commits

May 22, 2024 07:40


          feat: add words function to standard library

e8a1c06


          Update validity.rs

330e34e

kilianc marked this pull request as ready for review

May 22, 2024 14:50

Member

Ph0enixKM commented May 22, 2024

The branch does not pass the tests. It seems that the function inserts an additional new line symbol in the end of the string

Screenshot 2024-05-22 at 17 30 54

kilianc marked this pull request as draft

May 22, 2024 16:05

Contributor Author

kilianc commented May 22, 2024

Ty for letting me know, I also wonder if we should just slightly refactor split and then have words be split(a, ' ')

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

a5d399f

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

bfa1d51

Contributor Author

kilianc commented May 22, 2024

pub fun words(text: Text): [Text] {
  return split(text, " ")
}

Member

Ph0enixKM commented May 22, 2024

Yeah that would be interesting to see if we can do that. I tried to code the split function properly in bash in a relatively efficient way, but couldn't get it right.

Ph0enixKM reviewed

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

7e71a63

Co-authored-by: Phoenix Himself <[email protected]>

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

aff55d2

Contributor Author

kilianc commented May 22, 2024

Yeah that would be interesting to see if we can do that. I tried to code the split function properly in bash in a relatively efficient way, but couldn't get it right.

I tried locally and it works by just calling split, which version do you want to merge?

Member

Ph0enixKM commented May 22, 2024

I tried locally and it works by just calling split, which version do you want to merge?

Oh then let's merge the one with split - it's much cleaner

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

1b0ff5b

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

4ca58d2

kilianc commented

View reviewed changes

src/std/main.ab Outdated Show resolved Hide resolved


          Update src/std/main.ab

88f9051

kilianc marked this pull request as ready for review

May 22, 2024 22:57

Ph0enixKM approved these changes

View reviewed changes

Member

Ph0enixKM left a comment

LGTM

Ph0enixKM merged commit 6c3b5a1 into amber-lang:master

1 check passed

kilianc deleted the patch-1 branch

May 23, 2024 20:08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet