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

feat(stdlib): add new functions in std/text #373

Merged
merged 3 commits into from
Jul 31, 2024
Merged

feat(stdlib): add new functions in std/text #373

merged 3 commits into from
Jul 31, 2024

Conversation

MuhamedMagdi
Copy link
Contributor

@MuhamedMagdi MuhamedMagdi commented Jul 30, 2024 •
edited
Loading

  • reverse: reverses the given text.
  • starts_with: checks if a string is a prefix of another string.
  • ends_with: checks if a string is a suffix of another string.

The addition of starts_with & ends_with complement the existing contains function.

@MuhamedMagdi
feat(stdlib): add new functions in std/text
b97e8c1 
- `reverse`: reverses the given text.
- `is_prefix`: checks if a string is a prefix of another string.
- `is_suffix`: checks if a string is a suffix of another string.

These addition of `is_prefix` & `is_suffix` complement the existing `contains` function.
@Mte90
Copy link
Member

Mte90 commented Jul 30, 2024

I would prefer startswith and endswith like Python.
Let's see the others what think :-)

KrosFire
KrosFire approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@KrosFire KrosFire left a comment

Choose a reason for hiding this comment

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

LGTM

b1ek
b1ek requested changes Jul 30, 2024
View reviewed changes
Copy link
Member

@b1ek b1ek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

i agree with @Mte90, it should be named startswith and endswith to be more intuitive

the code is good tho, i pulled it and tested

Mte90
Mte90 approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@Mte90 Mte90 left a comment

Choose a reason for hiding this comment

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

Change is_suffix/is_prefix as suggest in the PR and we can merge it :-)

@MuhamedMagdi
Copy link
Contributor Author

shouldn't we use starts_with and ends_with as it's more appropriate to our naming convention?

@Mte90
Copy link
Member

Mte90 commented Jul 30, 2024

shouldn't we use starts_with and ends_with as it's more appropriate to our naming convention?

Indeed it is better!

@MuhamedMagdi
Copy link
Contributor Author

updated but i think there is something wrong with the date std function as they fail the checks.

@MuhamedMagdi
Copy link
Contributor Author

@Mte90 fixed #382

@b1ek b1ek merged commit 71224ec into amber-lang:master Jul 31, 2024
1 check failed
Mte90 pushed a commit to Mte90/Amber that referenced this pull request Sep 19, 2024
@MuhamedMagdi @Mte90
feat(stdlib): add new functions in std/text (amber-lang#373)
12c6b11 
* feat(stdlib): add new functions in `std/text`

- `reverse`: reverses the given text.
- `is_prefix`: checks if a string is a prefix of another string.
- `is_suffix`: checks if a string is a suffix of another string.

These addition of `is_prefix` & `is_suffix` complement the existing `contains` function.

* refactor: change `is_prefix` & `is_suffix` function names

is_prefix -> starts_with
is_suffix -> ends_with
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mte90 Mte90 Mte90 approved these changes

@b1ek b1ek b1ek approved these changes

@KrosFire KrosFire KrosFire approved these changes

@Ph0enixKM Ph0enixKM Awaiting requested review from Ph0enixKM

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MuhamedMagdi @Mte90 @b1ek @KrosFire