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

Fix Value Map widget with NULL value and field length < 38 #60801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Value Map widget with NULL value and field length < 38 #60801

wants to merge 1 commit into from

Conversation

agiudiceandrea
Copy link
Member

Description

Fixes #59368.

@agiudiceandrea agiudiceandrea added Regression Something which used to work, but doesn't anymore Bug Either a bug report, or a bug fix. Let's hope for the latter! backport release-3_40 backport release-3_42 labels Feb 28, 2025
@github-actions github-actions bot added this to the 3.44.0 milestone Feb 28, 2025
@agiudiceandrea agiudiceandrea mentioned this pull request Feb 28, 2025
Open
2 tasks
elpaso
elpaso reviewed Feb 28, 2025
View reviewed changes
Copy link
Contributor

@elpaso elpaso left a comment

Choose a reason for hiding this comment

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

LGTM, can you please add a test for this?

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 28, 2025 •
edited
Loading

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 69b90b4)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 69b90b4)

@agiudiceandrea
Copy link
Member Author

agiudiceandrea commented Feb 28, 2025 •
edited
Loading

LGTM, can you please add a test for this?

I succeeded to add new tests for #60786, but I failed here. Could you suggest me how to test this case? (I didn't find tests already in place in #57690)

@elpaso
Copy link
Contributor

elpaso commented Feb 28, 2025

LGTM, can you please add a test for this?

I succeeded to add new tests for #60786, but I failed here. Could you suggest me how to test this case? (I didn't find tests already in place in #57690)

did you look into testqgsvaluemapconfigdlg.cpp ?

@agiudiceandrea
Copy link
Member Author

did you look into testqgsvaluemapconfigdlg.cpp ?

Yes, I've already added a couple of tests there for #60786, but I can't figure out how to add a test for this case.

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

@elpaso elpaso elpaso left review comments

Assignees
No one assigned
Labels
backport release-3_40 backport release-3_42 Bug Either a bug report, or a bug fix. Let's hope for the latter! Regression Something which used to work, but doesn't anymore
Projects
None yet
Milestone
3.44.0
Development

Successfully merging this pull request may close these issues.

Value Map Widget NULL value breaks for text field shorter than 38 characters
2 participants
@agiudiceandrea @elpaso