Skip to content

Fixed missing emails in the author section. #7975

Open
renderman21 wants to merge 15 commits intoLMMS:masterfrom
renderman21:author_email
Open

Fixed missing emails in the author section. #7975
renderman21 wants to merge 15 commits intoLMMS:masterfrom
renderman21:author_email

Conversation

@renderman21
Copy link
Copy Markdown

As per #7936, here is the attempt in order to fix the missing emails as described.

Some things to note, most of the email format have the characters /dot/ and /at in them and that they had to be converted to the correct characters of . and @ respectively.

LADSPA plugins, however, are a different story. Originally, the solution was to manually change the email format but it did not work since it is dependent on the ladspa repository. Meaning, no changes can be made in , unless that repository is changed. Anyway, some author emails have a, rather, peculiar email format and that they've been manually changed. This case happens for the authors: Andy Wingo and Jesse Chappel. They both have spaces, and they are inconsistent with the other authors' emails present in the plugins/swh folder

@rubiefawn rubiefawn requested a review from messmerd October 22, 2025 21:38
Copy link
Copy Markdown
Member

@messmerd messmerd left a comment

Choose a reason for hiding this comment

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

Looks good code-wise

@rubiefawn rubiefawn linked an issue Nov 12, 2025 that may be closed by this pull request
1 task
@rubiefawn
Copy link
Copy Markdown
Contributor

rubiefawn commented Nov 12, 2025

Tested this, the emails for LADSPA plugins are still missing. The only change is that the full email addresses for Andy and Jesse are fully missing rather than partially missing. I assume this is because removing the spaces is allowing the full <foo@bar.net> to be interpreted as an invalid markup tag, whereas before <foo at bar dot net> would have been rendered as <foo.

Edit: It looks like the text is just fine, but now that there are no spaces in the email address, it is wrapping it to the next line, and the label itself is not resizing to accommodate and so the text gets clipped out of existence. Disabling text wrapping fixes this, but of course that means text can't wrap anymore.

@rubiefawn rubiefawn self-assigned this Nov 12, 2025
It wasn't utilizing the full width or height, which was especially apparent when resizing the window
This makes it more stylistically consistent with the native plugins by using rich text, and also fixes the stupid text wrapping issue.
@rubiefawn
Copy link
Copy Markdown
Contributor

The text wrapping issue was driving me crazy, so I changed the way LADSPA plugin descriptions are rendered to be more similar to how native plugin descriptions are rendered. This includes slightly more readable descriptions for LADSPA plugin properties for users who don't read the LADSPA spec for fun (LADSPA_PROPERTY_REALTIME, LADSPA_PROPERTY_INPLACE_BROKEN, LADSPA_PROPERTY_HARD_RT_CAPABLE).

This is a bit drastic and I thought about opening a new PR for this, but it's directly related to the line wrapping problem so...

Before

image
image

After

image

@rubiefawn rubiefawn added the needs testing This pull request needs more testing label Nov 12, 2025
const auto ldesc = lm->getDescription(lkey);

QString labelText =
"<p><b>" + QWidget::tr("Name") + ":</b> " + lm->getName(lkey) + "</p>"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"<p><b>" + QWidget::tr("Name") + ":</b> " + lm->getName(lkey) + "</p>"
QWidget::tr("<p><b>Name:</b> %1</p>").arg(lm->getName(lkey))

How about simplifying each line like this?

Things like the colon, spacing, and position of the name in relation to the "Name:" label are technically language-dependent.

Copy link
Copy Markdown
Contributor

@rubiefawn rubiefawn Nov 13, 2025

Choose a reason for hiding this comment

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

I agree regarding the colon and spacing, but I don't think including the markup in the translation string makes sense. What about something like:

Suggested change
"<p><b>" + QWidget::tr("Name") + ":</b> " + lm->getName(lkey) + "</p>"
"<p><b>" + QWidget::tr("Name: ") + "</b>" + lm->getName(lkey) + "</p>"

Edit: I forgot about QString::arg(), thanks for the reminder!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My concern was that an Arabic translation would look like this:

الاسم: Allpass delay line

with the "Allpass delay line" text being bold while the name label is non-bold.

The only easy way around this issue I think would be to include the markup in the translation string.

Copy link
Copy Markdown
Contributor

@rubiefawn rubiefawn Feb 7, 2026

Choose a reason for hiding this comment

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

I would imagine that the RTL text rendering happens after markup, since layout is dependent on markup, but I'll run a quick test to see what actually happens.

Edit: This can't currently be tested as there aren't actually translation strings for most of the plugin selector dialog. Hm.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After further experimentation, I can confirm that changing the language does not change the layout to RTL as I would have expected. While I do think that including markup in the translation string is generally incorrect, in this case I have to concede that it's the only way to get it working without fixing the greater RTL layout issue.

I renounce the savagery of `string + string + string + string...`
Also HTML-escape the plugin description as suggested by regulus79
Comment on lines +287 to +288
tr("Description: "), qApp->translate("PluginBrowser", descriptor.description).toHtmlEscaped(),
tr("Author: "), QString::fromUtf8(descriptor.author)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should the author come before the description?

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

Labels

bug gui needs testing This pull request needs more testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Effect Author string discards all characters after "<"

4 participants