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

Fix Load Data from CSV File for Value Map widget #60786

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

Conversation

agiudiceandrea
Copy link
Member

@agiudiceandrea agiudiceandrea commented Feb 27, 2025

Description

  • set UTF-8 codec when reading the CSV file

  • correctly handle CSV file with a single column without a trailing comma, like

1
2
3

while currently only a CSV like the following one is allowed

1,
2,
3,

Fixes #60765.

@github-actions github-actions bot added this to the 3.44.0 milestone Feb 27, 2025
@agiudiceandrea agiudiceandrea added Bug Either a bug report, or a bug fix. Let's hope for the latter! backport queued_ltr_backports Queued Backports backport release-3_42 labels Feb 27, 2025
Copy link

github-actions bot commented Feb 27, 2025

🪟 Windows builds

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

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 7b2415c)

@agiudiceandrea agiudiceandrea force-pushed the fix-60765-loaddatafromcsv branch from 1857f92 to 7b2415c Compare February 28, 2025 01:32
@agiudiceandrea agiudiceandrea changed the title [Forms] Fix Load Data from CSV File for Value Map widget Fix Load Data from CSV File for Value Map widget Feb 28, 2025
@@ -402,11 +404,11 @@ void QgsValueMapConfigDlg::loadMapFromCSV( const QString &filePath )
ceils << match.capturedTexts().last().trimmed().replace( QLatin1String( "\"\"" ), QLatin1String( "\"" ) );
}

if ( ceils.size() != 2 )
if ( ceils.size() == 0 )
Copy link
Member

Choose a reason for hiding this comment

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

@agiudiceandrea
Since the cells.size() > 2 is gone, just wanted to ask you if that's on purpose or if it's still safe.

Copy link
Member Author

@agiudiceandrea agiudiceandrea Mar 3, 2025

Choose a reason for hiding this comment

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

Thanks for the review!
If my logic isn't faulty, the QStringList ceils is populated in a previous while ( matches.hasNext() && ceils.size() < 2 ) loop, so it cannot have a size greater than 2: its size can only be either 0 or 1 or 2. If its size is 0, then outside loop will continue, if it's 1 then key = ceils[0] and val = QString( "" ), if it's 2 then key = ceils[0] and val = ceils[1].

Copy link
Member

Choose a reason for hiding this comment

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

I missed that part, thanks for the explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport queued_ltr_backports Queued Backports backport release-3_42 Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Load Data from CSV File into a Value Map Widget under a layers Properties > Attribute Form
2 participants