Show content types for tuples in tooltip
ClosedPublic

Authored by flherne on Jan 13 2017, 2:15 PM.

Details

Reviewers
brauch
Summary

(I'd quite like to put this in 5.1; the contents only being displayed in unsure-types is obviously a bug).

IndexedContainer::toString() did this, but the tooltips use DeclarationNavigationContext::htmlIdentifiedType() which didn't.

This led to the rather odd situation where unsure-types had *more* information:

a = b = 1, 2
b = "string"
a  # 'tuple'
b # 'unsure (tuple of ( int ), str)'

There's still quite a lot of duplication in this function, and with the toString() methods, that should be addressed at some point.

Incidentally, make the actual number of types used match the comment.

Test Plan

Tried it on things, it worked...

Diff Detail

Repository
R53 KDevelop: Python Support
Lint
Lint Skipped
Unit
Unit Tests Skipped
flherne updated this revision to Diff 10136.Jan 13 2017, 2:15 PM
flherne retitled this revision from to Show content types for tuples in tooltip.
flherne updated this object.
flherne edited the test plan for this revision. (Show Details)
flherne added a reviewer: brauch.
flherne set the repository for this revision to R53 KDevelop: Python Support.
Restricted Application added a subscriber: kdevelop-devel. ยท View Herald TranscriptJan 13 2017, 2:15 PM

BTW, i18n("%1 of ( %2 )") is copied from IndexedContainer::toString and isn't new.

I'd prefer it without spaces inside the parens (in both instances), but that would violate the string-freeze?

brauch accepted this revision.Jan 13 2017, 3:50 PM
brauch edited edge metadata.

Nice change, except for what's marked. Thanks!

duchain/navigation/declarationnavigationcontext.cpp
60

sorry, you can't put this in 5.1 if you introduce a new string :/
reuse one?

86

I think this needs to be translated

91

this needs a comment (i18nc)

duchain/navigation/declarationnavigationcontext.h
47

const&

This revision is now accepted and ready to land.Jan 13 2017, 3:50 PM

BTW, i18n("%1 of ( %2 )") is copied from IndexedContainer::toString and isn't new.

I'd prefer it without spaces inside the parens (in both instances), but that would violate the string-freeze?

Yes but you can do that specific change only in master.

flherne added inline comments.Jan 13 2017, 4:23 PM
duchain/navigation/declarationnavigationcontext.cpp
60

This is unchanged from the original (at line 85).

86

It's the same as in IndexedContainer::toString() (coincidentally at another line 85).

Could translate both, but that would be a new string unless it already exists somewhere.

91

Again, this string (including lack of comment) is directly from IndexedContainer::toString().

I'll do a follow-up patch in master as you suggest.

Ah, cool. Then just go ahead and yes, let's fix those issues in master. Thanks!

Ah, cool. Then just go ahead and yes, let's fix those issues in master. Thanks!