Problem/Motivation
See #2566503: [meta] Replace remaining !placeholder for Non-URL HTML outputs only
search_excerpt
Proposed resolution
search_excerpt()
contains an instance of a !placeholder
, but there is no corresponding argument passed for !placeholder
to be replaced with. The string is only used as a way for search to try to get information on how a translator would translate an ellipsis. Therefore, whether it uses !placeholder
or @placeholder
makes no difference.
So, it can be simply changed from one to the other.
Remaining tasks
A followup is needed because the current code in search_excerpt()
is incompatible with some translation techniques, including adding markup to the string, and it has hardcoded assumptions about what a string of excerpts would look like in a given language.
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#3 | 2568611.3.patch | 743 bytes | alexpott |
Comments
Comment #2
alexpottThis one is interesting. It's not using
!placeholder
as a placeholder :)...It is using it as a way to allow translators to translate the multiple
…
.Comment #3
alexpottI guess though there is no harm in replacing ! with @
Comment #4
dawehnerHaha, not at all, but yeah it makes sense I could imagine something like "..." depends on the language, or at least could be useful to be translatable.
Comment #5
catchInteresting one.
Committed/pushed to 8.0.x, thanks!
Comment #8
alexpottpifr is slow.
Comment #9
dawehnerSeriously testbot
Comment #10
xjmSo I was really concerned that this change went in without testing, until @alexpott pointed out that there is no variable passed to placeholders at all, and
SafeMarkup
will never even be used because there are no placeholder arguments.However, the reason I asked for this issue to be split out in the first place is that there is serious code smell in this function related to its use of
t()
and otherwise. I was hoping it would be discussed in this issue, but I think we at least need an inline comment with thatt()
documenting why it's used and ideally linking the followup to fix the whole function. @catch located #2212323: Make a way to better control search result display, but there is also a specific bug/problem WRT the translatability.Comment #11
alexpottComment #12
jhodgdonI don't understand what the translation problem is. Can you please file an issue explaining what the "code smell" you detected is?
The thing is, the search_excerpt() function needs a way to output
... Some text from a node ... some more text ... some more text ... some more text ...
and to allow other languages to perhaps use a more appropriate character than ... to separate the excerpts.
So that is why we do t(), as documented in the comment directly above the line changed in this patch:
I do not know of any other way to let translators provide the appropriate character(s) except by calling t()... do you?