The function getAutocompleteFilter controls when we show an autocomplete popup in composing message content. It's currently pretty convoluted and hard to follow, as highlighted in #3290. I see that as the root cause of issue #3289, and I think there are more issues lurking in that logic that are hard to see because of the complexity of the code. So I'd love to see a followup to make it simpler and clearer.
Concretely: There are a lot of conditionals in it that are various special cases. Also it's not at all clear what the various pieces are supposed to mean. (lastWordPrefix doesn't really have anything to do with a word, let alone a last word. And lastIndex isn't particularly the last index of anything.)
To fix it, I think the main steps are:
Write jsdoc saying what the function is supposed to mean. What do the return values mean? In particular, what return values signify that there's nothing to autocomplete?
Part of this answer will be complicated to explain. That points at the next step, improving the interface so it's less complicated. In the actual branch, the jsdoc can come after the better interface.
Improve the return value interface. Replace lastWordPrefix with a meaningful name, e.g. code or sigil. In the case where there's nothing to autocomplete, return null rather than a mishmash of partial values.
Think about the edge cases in the logic, and adjust them.
What is the condition ['\n', ' ', '#', ':'].includes(text[text.lastIndexOf('@') - 1]) doing? Are there similar cases that should also be included? How can we tell when we're done adding cases there? What does the webapp do?
What is the condition !['\n', ' '].includes(text[lastIndex + 1]) doing? Again, are there similar cases that should also be included? How can we tell when we're done adding cases? And what does the webapp do?
The function
getAutocompleteFilter
controls when we show an autocomplete popup in composing message content. It's currently pretty convoluted and hard to follow, as highlighted in #3290. I see that as the root cause of issue #3289, and I think there are more issues lurking in that logic that are hard to see because of the complexity of the code. So I'd love to see a followup to make it simpler and clearer.Concretely: There are a lot of conditionals in it that are various special cases. Also it's not at all clear what the various pieces are supposed to mean. (
lastWordPrefix
doesn't really have anything to do with a word, let alone a last word. AndlastIndex
isn't particularly the last index of anything.)To fix it, I think the main steps are:
lastWordPrefix
with a meaningful name, e.g.code
orsigil
. In the case where there's nothing to autocomplete, returnnull
rather than a mishmash of partial values.m
flag.['\n', ' ', '#', ':'].includes(text[text.lastIndexOf('@') - 1])
doing? Are there similar cases that should also be included? How can we tell when we're done adding cases there? What does the webapp do?!['\n', ' '].includes(text[lastIndex + 1])
doing? Again, are there similar cases that should also be included? How can we tell when we're done adding cases? And what does the webapp do?