Skip to content

Fix Raw Image Handling and Improve Text File Encoding Compatibility #233

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

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

CyanVoxel
Copy link
Member

Some small bugfixes and improvements involving thumbnail and preview rendering:

  • Fix RAW images not being loaded correctly in the preview panel
  • Fix trying to read size data from null images
  • Refactor os.stat to <Path object>.stat()
  • Remove unnecessary upper/lower conversions
  • Improve encoding compatibility beyond UTF-8 when reading text files
  • Misc. code cleanup

- Fix RAW images not being loaded correctly in the preview panel
- Fix trying to read size data from null images
- Refactor `os.stat` to `<Path object>.stat()`
- Remove unnecessary upper/lower conversions
- Improve encoding compatibility beyond UTF-8 when reading text files
- Code cleanup
@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Type: Refactor Code that needs to be restructured or cleaned up labels Jun 1, 2024
@CyanVoxel CyanVoxel added this to the Alpha 9.3 milestone Jun 1, 2024
Comment on lines 20 to 21
for encoding in ENCODINGS:
with open(filepath, "r", encoding=encoding) as text_file:
Copy link
Contributor

@yedpodtrzitko yedpodtrzitko Jun 1, 2024

Choose a reason for hiding this comment

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

I dont think opening the file (up to) 5 times it's a good approach, even though utf-8 might be the right choice in majority of cases.

I'd probably use existing library like chardet.

If you want to do it without introducing another dependency, opening the file once in a binary mode and then trying to do the content decode might also work (code not tested):

with open(file_path, 'rb') as text_file: # `rb` opens the file in binary mode 
    text_file = file.read(1024)  # Read the first 1024 bytes

for encoding in ENCODINGS:
    try:
        decoded_data = raw_data.decode(encoding, errors='replace')
        if '�' not in decoded_data:
            return encoding
    except Exception:
        continue

and then it could be worth saving the file encoding somewhere in the library, so it doesnt need to be done every time 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I was on the fence about adding another dependency, but I think I needed to hear it from someone else to realize that it's the better approach. I'll go and get a chardet implementation going 👍

Also, saving the encoding somewhere would definitely be the way to go. Probably out of the scope of this PR, but that would be very useful to have on hand along with cached file stats.

@CyanVoxel CyanVoxel merged commit 0646508 into main Jun 3, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as intended Type: Refactor Code that needs to be restructured or cleaned up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants