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

csl.api: Fix GsfSemanticLayer after 0ba2e7a43595d2a267c1fda58b469e6e947f8830 #8103

Merged
merged 1 commit into from
Jan 1, 2025
Merged

csl.api: Fix GsfSemanticLayer after 0ba2e7a43595d2a267c1fda58b469e6e947f8830 #8103

merged 1 commit into from
Jan 1, 2025
+76 −5

Conversation

matthiasblaesing
Copy link
Contributor

The implementation of GsfSemanticLayer#firstSequenceElement returns wrong/out-of-range values for the first element. This in turn causes tooltip/rendering errors when the sequence is used/positioned.

Closes: #8102

@matthiasblaesing
csl.api: Fix GsfSemanticLayer after 0ba2e7a
40467c8 
The implementation of GsfSemanticLayer#firstSequenceElement returns
wrong/out-of-range values for the first element. This in turn causes
tooltip/rendering errors when the sequence is used/positioned.

Closes: apache#8102
@matthiasblaesing matthiasblaesing added the CSL [ci] enable web job label Dec 31, 2024
@apache apache locked and limited conversation to collaborators Dec 31, 2024
@apache apache unlocked this conversation Dec 31, 2024
@matthiasblaesing matthiasblaesing added the ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) label Dec 31, 2024
@apache apache locked and limited conversation to collaborators Dec 31, 2024
@apache apache unlocked this conversation Dec 31, 2024
@matthiasblaesing matthiasblaesing mentioned this pull request Dec 31, 2024
Closed
@apache apache locked and limited conversation to collaborators Dec 31, 2024
@apache apache unlocked this conversation Dec 31, 2024
lkishalmi
lkishalmi approved these changes Dec 31, 2024
View reviewed changes
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Seems to be good.

If I get this right, the original implementation based on binary search were wrong, as it gave back the "insert" position of the offset, instead of the position of the element before that offset.

@lkishalmi
Copy link
Contributor

Thank you @matthiasblaesing !, I was completely on wrong track tracing this!

junichi11
junichi11 approved these changes Jan 1, 2025
View reviewed changes
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

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

I've verified the problem is fixed. The unit test is so nice. Thank you!

@matthiasblaesing
Copy link
Contributor Author

@lkishalmi @junichi11 thank you both.

@matthiasblaesing matthiasblaesing merged commit 59bea23 into apache:master Jan 1, 2025
62 of 90 checks passed
@matthiasblaesing matthiasblaesing deleted the gh-8102 branch January 1, 2025 14:00
@junichi11 junichi11 added this to the NB25 milestone Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lkishalmi lkishalmi lkishalmi approved these changes

@junichi11 junichi11 junichi11 approved these changes

Assignees
No one assigned
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) CSL [ci] enable web job
Projects
None yet
Milestone
NB25
Development

Successfully merging this pull request may close these issues.

[CSL] Unused tooltip is not shown
3 participants
@matthiasblaesing @lkishalmi @junichi11