Changeset View
Standalone View
plugins/qmljs/codecompletion/context.cpp
Show First 20 Lines • Show All 450 Lines • ▼ Show 20 Line(s) | 448 | case QmlJSGrammar::T_RPAREN: | |||
---|---|---|---|---|---|
451 | } | 451 | } | ||
452 | break; | 452 | break; | ||
453 | case QmlJSGrammar::T_IDENTIFIER: | 453 | case QmlJSGrammar::T_IDENTIFIER: | ||
454 | case QmlJSGrammar::T_DOT: | 454 | case QmlJSGrammar::T_DOT: | ||
455 | case QmlJSGrammar::T_THIS: | 455 | case QmlJSGrammar::T_THIS: | ||
456 | break; | 456 | break; | ||
457 | case QmlJSGrammar::T_COMMA: | 457 | case QmlJSGrammar::T_COMMA: | ||
458 | stack.top().commas++; | 458 | stack.top().commas++; | ||
459 | break; | ||||
aaronpuchert: Looks also suspicious. | |||||
So operator{Start,End} are used in functionCallTips to find the m_typeToMatch. The idea seems to be that if we write a + then we want something of the same type as a to add to it. With the fallthrough we apply this for commas as well, so we try to match the type before the comma. That doesn't seem right. I'm actually inclined to insert a break here. Looking at change R32:699603b11f3664e9e386d32add8496a0d3a8a7cc where this was introduced, it might have been an oversight. @brauch Can you comment on that? The original author of the code doesn't seem to be on Phabricator, but you have apparently worked on QmlJs as well. aaronpuchert: So `operator{Start,End}` are used in `functionCallTips` to find the `m_typeToMatch`. The idea… | |||||
Since this hunk is actually changing behavior, I'd commit it in a separate patch and also explain properly in the commit msg. All other hunks are mainly cosmetic iiuc and can be committed in bulk. kfunk: Since this hunk is actually changing behavior, I'd commit it in a separate patch and also… | |||||
@aaronpurchert This fix would be also candidate for 5.3 branch, right? If so, please cherry-pick -x it there, or tell me, I would do it then. kossebau: @aaronpurchert This fix would be also candidate for 5.3 branch, right? If so, please cherry… | |||||
kossebau: Cherry-picked now to 5.3, so can be checked off :) | |||||
459 | default: | 460 | default: | ||
460 | // The last operator of every sub-expression is stored on the stack | 461 | // The last operator of every sub-expression is stored on the stack | ||
461 | // so that "A = foo." can know that attributes of foo having the same | 462 | // so that "A = foo." can know that attributes of foo having the same | ||
462 | // type as A should be highlighted. | 463 | // type as A should be highlighted. | ||
463 | stack.top().operatorStart = lexer.tokenStartColumn() - 1; | 464 | stack.top().operatorStart = lexer.tokenStartColumn() - 1; | ||
464 | stack.top().operatorEnd = lexer.tokenEndColumn() - 1; | 465 | stack.top().operatorEnd = lexer.tokenEndColumn() - 1; | ||
466 | break; | ||||
465 | } | 467 | } | ||
466 | } | 468 | } | ||
467 | 469 | | |||
468 | return stack; | 470 | return stack; | ||
469 | } | 471 | } | ||
470 | 472 | | |||
471 | DeclarationPointer CodeCompletionContext::declarationAtEndOfString(const QString& expression) | 473 | DeclarationPointer CodeCompletionContext::declarationAtEndOfString(const QString& expression) | ||
472 | { | 474 | { | ||
Show All 34 Lines |
Looks also suspicious.