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

Fix newlines in subject being shown with ? #4298

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Yutsuten
Copy link
Member

@Yutsuten Yutsuten commented May 11, 2024

  • What does this PR do?
  1. Keeps env->subject with the original content, the previous code was replacing LF characters with ?.
    1. Other fields may be affected, but I think that if any additional fixes become needed, it should be done in format_string() or similar
  2. Sanitize env->subject into env->disp_subj by replacing whitespace characters (iswspace()) with a space character.

I also tested with and without a subjectrx configuration, seems to be working as expected.

There are also some side changes as discussed in #4276

  1. #ifdef not needed anymore in expando/format.c

As iswspace() is a bigger set than iswblank(), we can drop iswblank().

  1. Return value of subjrx_apply_mods from bool to void. Return value is not used anywhere
  • Screenshots (if relevant)

Subject with newlines before the change:

???      ? ? ?    ?  ?  ?  ?   ??Google Cloud Platform & APIs...

Subject with newlines after the change:

                                 Google Cloud Platform & APIs...

This is my first PR with C language related changes. Please tell me if there are any security or performance issues in my code.

Thank you @flatcap for giving me so much information in the issue.

Also please tell me if it is better to squash every commit into one at the end of the reviews.

@Yutsuten Yutsuten self-assigned this May 11, 2024
@Yutsuten Yutsuten requested a review from a team as a code owner May 11, 2024 16:15
@flatcap
Copy link
Member

flatcap commented May 12, 2024

Thanks!
I shall have a read and a play as soon as I can.

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