8000 [fix bug]-cannot use a str to initialize an array with typecode 'B' by jinxiangwzh · Pull Request #187 · python-xlib/python-xlib · GitHub
[go: up one dir, main page]
Skip to content

[fix bug]-cannot use a str to initialize an array with typecode 'B' - #187

Open
jinxiangwzh wants to merge 1 commit into
python-xlib:masterfrom
jinxiangwzh:master
Open

[fix bug]-cannot use a str to initialize an array with typecode 'B'#187
jinxiangwzh wants to merge 1 commit into
python-xlib:masterfrom
jinxiangwzh:master

Conversation

@jinxiangwzh
@jinxiangwzh jinxiangwzh commented Dec 21, 2020
Copy link
Copy Markdown

No description provided.

@petli
petli commented Dec 21, 2020 •
edited
Loading
Copy link
Copy Markdown
Contributor

Thank you for the PR, but this change breaks a lot of unit tests (see the CI build failure), so can you look at that and see what you need change in the code to ensure they still work, please?

I'm also not sure what the bug is here. Could you explain to which methods or requests you want to pass in a string to this structure, rather than controlling the encoding of it to an array yourself? I'm thinking that such encoding could make more sense in the wrapping methods in the X objects (Display, Window etc), rather than this low down in the protocol implementation.

@Avasam
Avasam commented Nov 1, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

I'm going through open PRs while preparing my fork for initial release, and I think this one can be closed rather than merged.

Your changes run val.encode() for every value that is not bytes. The usual input for PropertyData is a list or tuple of ints, for example change_property(atom, Xatom.CARDINAL, 32, [1, 2, 3]). With this change, every call like that fails with AttributeError: 'list' object has no attribute 'encode'. For format 16 and 32 the change is wrong even for str input. The bytes from str.encode() get reinterpreted as 16 or 32 bit items, and frombytes raises ValueError when the length is not a multiple of the item size.

The library already handles the case this was meant for. For text, use Window.change_text_property(property, property_type, data). It accepts str and encodes it based on the property type: latin-1 for STRING, UTF-8 for UTF8_STRING. For any other property type, encode the string yourself and pass bytes to change_property, which PropertyData already accepts.

Since change_text_property covers this, I'd suggest just closing the PR.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jinxiangwzh @petli @Avasam @vasily-v-ryabov
0