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

Allow custom paths in Discord avatars #107

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Allow custom paths in Discord avatars #107

merged 1 commit into from
Mar 12, 2024

Conversation

mlomb
Copy link
Owner

@mlomb mlomb commented Feb 19, 2024

This will load avatars from local directories if exports have media files enabled. Fixes #106

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (61a8f06) 71.90% compared to head (fba64b2) 71.90%.

Files Patch % Lines
pipeline/parse/parsers/DiscordParser.ts 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
- Coverage   71.90%   71.90%   -0.01%     
==========================================
  Files          60       60              
  Lines        2442     2445       +3     
  Branches      514      516       +2     
==========================================
+ Hits         1756     1758       +2     
- Misses        626      627       +1     
  Partials       60       60              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This will load avatars from local directories if exports have media files enabled.
Fixes #106
@mlomb mlomb force-pushed the discord-local-avatars branch from a4c2f43 to fba64b2 Compare February 19, 2024 19:25
Copy link

⚡ Preview for this PR: https://pr-107.chat-analytics.pages.dev
📊 Demo

@hopperelec
Copy link
Contributor

| Discord | `json` from [DiscordChatExporter](https://github.com/Tyrrrz/DiscordChatExporter) ||||| ✅ (until link expires) | ✅ (as text) ||

(until link expires)

This should maybe be updated. Of course, the link could still "expire" if the user deletes the media, but I think there should be some sort of mention of the difference between how media is handled when media is/isn't used

// assume it's a custom avatar stored as media
// store the full path to the avatar
// note that this will not work unless the user puts the report.html in the correct
// folder, but since we don't have the online URL, we do this as best-effort
Copy link
Contributor

Choose a reason for hiding this comment

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

The user should probably be informed of this in some way, perhaps in the header of the report similarly to the header used for the demo. Also, once configuration is added, the user should have the option to embed images in the report (both with and without media, since Discord profile picture links expire)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it should be a warning after the generation process. Showing a header like the demo that can't be easily removed will be annoying.
Embedding images is also complicated. With links it's not possible, since we may have to download thousands of images off Discord servers at once, which is bad. With local files, it may be possible but there isn't an easy way right now and I don't think it's worth it.

@mlomb mlomb merged commit 557ab64 into main Mar 12, 2024
9 checks passed
@mlomb mlomb deleted the discord-local-avatars branch March 12, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discord - Locally stored users profile pictures not working
2 participants