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

751 Expose isEmpty and isBlank for EL evaluation #752

Merged
merged 5 commits into from
Jul 16, 2024

Conversation

Positronic-Brain
Copy link
Contributor

This would implement issue #751

Copy link
Member

@kwin kwin left a comment

Choose a reason for hiding this comment

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

Looks good in general, but:

  • requires a minor version upgrade (please adjust the pom.xml accordingly)
  • requires some info on documentation since when which method is supported.

docs/AdvancedFeatures.md Outdated Show resolved Hide resolved
@ghenzler
Copy link
Member

there isn't much that speaks against linking these two methods, but we could also just document the empty operator that EL provides out of the box (see https://docs.oracle.com/cd/E17802_01/j2ee/j2ee/1.4/docs/tutorial-update6/doc/JSPIntro7.html#wp71089)

@Positronic-Brain
Copy link
Contributor Author

Yeah, empty is a bit redundant, but since I was already exposing "isBlank" I decided to add its related method for consistency.

@Positronic-Brain Positronic-Brain requested a review from kwin July 16, 2024 09:22
@ghenzler
Copy link
Member

@Positronic-Brain so I just checked and empty ' ' really does not return true (which I thought would be the case for EL). So fair enough to add the two methods, but I probably would also add https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#defaultIfBlank-T-T- then (as we have defaultIfEmpty(..) already)

@Positronic-Brain
Copy link
Contributor Author

@Positronic-Brain so I just checked and empty ' ' really does not return true (which I thought would be the case for EL). So fair enough to add the two methods, but I probably would also add https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#defaultIfBlank-T-T- then (as we have defaultIfEmpty(..) already)

Sounds reasonable. Will do.

Copy link

Copy link
Member

@kwin kwin left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks a lot.

@kwin kwin merged commit 71ccfd8 into develop Jul 16, 2024
19 checks passed
@kwin kwin deleted the feature/751-expose-isEmpty-and-isBank-to-EL branch July 16, 2024 17:06
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.

3 participants