Skip to content

Fix h5ls issue with quoting when displaying integer data as ASCII characters - #6553

Merged
jhendersonHDF merged 3 commits into
HDFGroup:developfrom
jhendersonHDF:h5ls_ascii_quote_fix
Aug 7, 2026
Merged

Fix h5ls issue with quoting when displaying integer data as ASCII characters#6553
jhendersonHDF merged 3 commits into
HDFGroup:developfrom
jhendersonHDF:h5ls_ascii_quote_fix

Conversation

@jhendersonHDF

Copy link
Copy Markdown
Collaborator

When using the -r option to have h5dump display 1-byte integer datasets and attributes as ASCII characters, the data display logic includes a section that deals with including a closing double-quote character for the data values. This logic wasn't carried over to h5ls for its similar -s option, causing it to drop the closing double-quote:

#include "hdf5.h"

int
main(int argc, char **argv)
{
    hid_t file_id;
    hid_t dset_id;
    hid_t type_id;
    hid_t space_id;
    hid_t attr_id;
    hsize_t dims[] = { 81 };
    /* 81-byte string, including NUL terminator */
    char *data = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012345678901234567";

    file_id = H5Fcreate("testchar.h5", H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);

    type_id = H5Tcopy(H5T_NATIVE_UCHAR);

    space_id = H5Screate_simple(1, dims, NULL);

    dset_id = H5Dcreate2(file_id, "dset", type_id, space_id,
                         H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);

    H5Dwrite(dset_id, type_id, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);

    attr_id = H5Acreate2(dset_id, "attr", type_id, space_id,
                         H5P_DEFAULT, H5P_DEFAULT);

    H5Awrite(attr_id, type_id, data);

    H5Sclose(space_id);
    H5Tclose(type_id);
    H5Aclose(attr_id);
    H5Dclose(dset_id);
    H5Fclose(file_id);

    return 0;
}
h5ls -ds testchar.h5
dset                     Dataset {81}
    Data:
         "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012345678901234567\000

These changes restore the double-quote for h5ls and refactor the logic for h5dump to fix some other similar formatting issues when data elements are wrapped to new lines according to the tool's column limit setting.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

✅ All areas have been signed off.

@jhendersonHDF
jhendersonHDF force-pushed the h5ls_ascii_quote_fix branch 2 times, most recently from bcbfa6c to c17486f Compare July 23, 2026 01:15
@jhendersonHDF
jhendersonHDF marked this pull request as ready for review July 23, 2026 01:59
@hyoklee

hyoklee commented Jul 23, 2026

Copy link
Copy Markdown
Member

What's the issue number for this PR?

hyoklee
hyoklee previously approved these changes Jul 23, 2026
Comment thread release_docs/CHANGELOG.md

## Tools

### Fixed an issue with quoting of data values in h5ls and h5dump when displaying as ASCII characters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

quoting of data values -> data value quoting

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I find the former better to read since the issue was with quoting and not "data value XXX"

Comment thread release_docs/CHANGELOG.md Outdated
Comment thread release_docs/CHANGELOG.md Outdated

When using the `-s` (h5ls) or `-r` (h5dump) option to display 1-byte integer datasets and
attributes as ASCII characters, a closing double-quote character for data values was being
dropped in some cases. This double-quote character has been restored and similar formatting

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

has been restored and similar -> has been restored, and similar

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is at best a preference

Comment thread release_docs/CHANGELOG.md Outdated
Comment thread release_docs/CHANGELOG.md Outdated
attributes as ASCII characters, a closing double-quote character for data values was being
dropped in some cases. This double-quote character has been restored and similar formatting
issues have been fixed for cases where elements are wrapped to new lines according to the
particular tool's column limit setting.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove particular.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Particular here is to distinguish between h5ls and h5dump, as each tool has its own column limit setting

@jhendersonHDF

Copy link
Copy Markdown
Collaborator Author

What's the issue number for this PR?

No issue currently, as the problem was reported external to GitHub

Comment thread tools/lib/h5tools_dump.c
status = h5tools_dump_mem(stream, &string_dataformat, &datactx, obj_id);
}
}
if (datactx.display_char && H5Tget_size(f_type) == 1 && H5Tget_class(f_type) == H5T_INTEGER) {

@jhendersonHDF jhendersonHDF Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This logic was added so that h5dump adds the closing quote character when displaying the data as ASCII characters, but it goes against the general tools architecture and complicates the logic. The fix that follows the tools architecture, allowing h5tools_simple_prefix() to print the closing quote character, is below.

Comment thread tools/src/h5ls/h5ls.c
}

/* Output closing line suffix character when printing 1-byte integer data as ASCII */
if (info->ascii) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Since h5ls doesn't use the h5tools_dump_data() function from the tools library, it needs its own specific logic to cover this case.

hyoklee
hyoklee previously approved these changes Jul 23, 2026
Comment thread release_docs/CHANGELOG.md
issues have been fixed for cases where elements are wrapped to new lines according to the
particular tool's column limit setting.
attributes as ASCII characters, a closing double-quote character for data values was dropped
in some cases. This double-quote character has been restored and similar formatting issues

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comma. has been restored and similar -> has been restored, and similar

@jhendersonHDF jhendersonHDF added this to the HDF5 2.3.0 milestone Jul 24, 2026
Comment thread tools/test/h5dump/h5dumpgentest.c Outdated
lrknox
lrknox previously approved these changes Aug 4, 2026
@brtnfld
brtnfld requested review from mattjala and removed request for bmribler August 4, 2026 02:47
mattjala
mattjala previously approved these changes Aug 5, 2026
@jhendersonHDF
jhendersonHDF dismissed stale reviews from mattjala and lrknox via 4223d69 August 5, 2026 15:47
@jhendersonHDF

Copy link
Copy Markdown
Collaborator Author

Fixed the conflict in CHANGELOG.md

@github-actions
github-actions Bot requested review from mattjala and removed request for hyoklee and mattjala August 5, 2026 17:24

@lrknox lrknox left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It appears you've considered previous suggestions for the CHANGELOG.md entry. Most CHANGELOG.md entries are a single line for the description part of the entry; you might consider changing that, or it can be done later if necessary.

@jhendersonHDF

Copy link
Copy Markdown
Collaborator Author

It appears you've considered previous suggestions for the CHANGELOG.md entry. Most CHANGELOG.md entries are a single line for the description part of the entry; you might consider changing that, or it can be done later if necessary.

I applied a few of the suggestions, but don't particularly agree with the remaining ones so left them alone.

@jhendersonHDF
jhendersonHDF merged commit 57128d3 into HDFGroup:develop Aug 7, 2026
131 of 133 checks passed
@github-project-automation github-project-automation Bot moved this from To be triaged to Done in HDF5 - TRIAGE & TRACK Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants