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

profiles: claws-mail: add cache support #6378

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

glitsj16
Copy link
Collaborator

Add support for the (widely used) Fancy plugin:
Renders HTML e-mail using the WebKit library

https://www.claws-mail.org/plugins.php

Related to #6377.

Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

email-common contains private-cache, so wouldn't these changes be a no-op?

Does the plugin not work with the current profile?

@glitsj16
Copy link
Collaborator Author

email-common contains private-cache, so wouldn't these changes be a no-op?

You're absolutely right. This PR makes no sense, my bad.

@glitsj16 glitsj16 closed this Jun 12, 2024
@glitsj16 glitsj16 deleted the cm-cache branch June 12, 2024 12:20
@kmk3
Copy link
Collaborator

kmk3 commented Jun 13, 2024

email-common contains private-cache, so wouldn't these changes be a no-op?

You're absolutely right. This PR makes no sense, my bad.

I was confused about the plugin not working without the changes, but the
changes themselves seem fine (if they are indeed a no-op with private-cache).

It seems good for documentation purposes, to have it blocked in
disable-programs and to make it easier to use the real cache directory (by just
using ignore private-cache).

@kmk3
Copy link
Collaborator

kmk3 commented Jun 18, 2024

@glitsj16

Hello, do you want to continue with this PR?

It's likely possible to click to restore the branch and then to reopen the PR.

@glitsj16
Copy link
Collaborator Author

@kmk3

Hi, I'll take care of it today. Last days of school exams here, less computer-time for me usually. But it shouldn't take that long to restore and reopen this. Thanks for the reminder!

@glitsj16 glitsj16 restored the cm-cache branch June 18, 2024 11:38
@glitsj16 glitsj16 reopened this Jun 18, 2024
@kmk3
Copy link
Collaborator

kmk3 commented Jun 18, 2024

Hi, I'll take care of it today. Last days of school exams here, less
computer-time for me usually. But it shouldn't take that long to restore and
reopen this. Thanks for the reminder!

Ah I see, I wasn't sure if you had seen previous comment, so I commented again
just in case.

Anyway, no hurry on the PR.

Good luck on the exams!

Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

Why the new changes?

It seems like the version from yesterday would work and make it easier to use
the real cache directory if desired.

@glitsj16
Copy link
Collaborator Author

@kmk3

Why the new changes?

Oh, I misunderstood. Thought you'd only wanted to keep the disable-common.inc part and leave private-cache as-is. To avoid more mishaps from my side, do you think it would make more sense to ignore private-cache for CM and use the dedicated whitelisted ~/.cache/claws-mail instead?

@kmk3
Copy link
Collaborator

kmk3 commented Jun 18, 2024

To avoid more mishaps from my side, do you think it would make more sense to
ignore private-cache for CM and use the dedicated whitelisted
~/.cache/claws-mail instead?

I think in general it's good to leave private-cache on by default to help
keep sandboxes isolated and to avoid unecessarily reducing the lifespan of
storage devices, as IME programs rarely make good use of ~/.cache.

The exceptions would be if the program creates a giant cache (which like
private-opt could take too much RAM) or when caching actually makes a
positive noticeable difference (like shader caches in certain programs).

Edit: To be clear, I think that the original changes were correct (with the
whitelist and without ignore private-cache).

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.

None yet

2 participants