remove prefix and update symbol#1198
Conversation
lionel-
left a comment
There was a problem hiding this comment.
Looks good!
Just need to discuss/revert this before merging:
the Type Parameter visually maps well as it is a T in angle brackets.
I would revert back to Function or maybe Method because:
-
That's what most LSPs do. Although arguably most languages use decorated functions for tests, which isn't the case for testthat.
-
It would interfere with symbol discovery when filtering symbols. If you're looking for a type, it would be surprising to find
test_that()calls. Less so if you're looking for a function.
|
Fair, I was going more after visual queues than semantics of the symbol. If we are going for semantics I feel that function/method is not right either as this is really a call/expression. However, nothing in that vein is available to us. So, I'm fine reverting to using the old function though I do feel it would be nice if the tests were visually separated from helper functions etc now that the prefix is gone. Your call |
|
Maybe Method would convey that separation? |
|
Well, sadly they use the same icon 🫠 Still, it's not a blocker for me so I'll update to use that... Unless you are ok with something like Object which is visually distinct and sorta correct in that an expression is an object of sorts.. |
|
Let's just use Function I think. Semantically, |
Fix #995
This is just to get my feet wet with the smallest of issues, even though the issue asked for some discussion around it.
This PR removes the "Test: " prefix in the outline and instead uses a different icon to differentiate. There is no perfect symbol kind for tests but the Type Parameter visually maps well as it is a T in angle brackets. If we don't like this conflation of symbols we can revert