DEV2-4200: Fix inconsistent behavior (also DEV2-3330) #692
Conversation
ofekby
force-pushed
the
fix-inconsistent-behavior
branch
from
302859b to
e57eaa3
Compare
| import com.tabnineCommon.general.TopicBasedState | ||
| import java.util.function.Consumer | ||
|
|
||
| class BinaryStateSingleton private constructor() : |
There was a problem hiding this comment.
should we consider this singleton to be persisted, til first poling?
will that make the experience more consistent?
There was a problem hiding this comment.
I think persisting it can only cause more flakiness and unstable states. Data in the state is considered to be very moment related. Also few IDE can edit it simultaneously.
| val alphaEnabled = CapabilitiesService.getInstance().isCapabilityEnabled(Capability.ALPHA) | ||
| val chatCapabilityEnabled = | ||
| CapabilitiesService.getInstance().isCapabilityEnabled(Capability.TABNINE_CHAT) |
There was a problem hiding this comment.
how does this behave in SH? empty responses?
There was a problem hiding this comment.
It doesn't execute this code in SH, this code is only for the SAAS plugin. Anyhow the Capabilities service returns true for any capability asked in SH (It always was that way, didn't change it).
| val chatCapabilityEnabled = | ||
| CapabilitiesService.getInstance().isCapabilityEnabled(Capability.TABNINE_CHAT) | ||
|
|
||
| if (isLoggedIn && (alphaEnabled || chatCapabilityEnabled)) { |
There was a problem hiding this comment.
does this apply to SH too? how does this work?
There was a problem hiding this comment.
No, this code is the enable logic of the SAAS, SH has separate class for its logic.
| when { | ||
| !CapabilitiesService.getInstance().isCapabilityEnabled(Capability.FORCE_REGISTRATION) -> return | ||
| !forceRegistration -> return |
There was a problem hiding this comment.
is this still relevant?
There was a problem hiding this comment.
Yes, the force registration flag is still alive
| @@ -44,17 +45,23 @@ fun openSigninPage() { | |||
| fun presentGreeting(state: StateResponse?) { | |||
| if (state?.userName != null) { | |||
| TABNINE_NOTIFICATION_GROUP | |||
| .createNotification("You are signed in as ${state.userName}", NotificationType.INFORMATION) | |||
| .createNotification( | |||
| "You are signed in as ${state.userName}", | |||
There was a problem hiding this comment.
question. do we have the team name too?
There was a problem hiding this comment.
No, the state doesn't contain the team name or any information about the team.
| capabilities -> { | ||
| if (capabilities.isReady()) { | ||
| boolean newForceRegistration = capabilities.isEnabled(Capability.FORCE_REGISTRATION); | ||
| CapabilitiesStateSingleton.getInstance() |
There was a problem hiding this comment.
do all IJ windows share this singleton? or do they run in different processes?
There was a problem hiding this comment.
No, its per IJ process. Every IJ window is a separate process.
There was a problem hiding this comment.
so will these changes solve also the multiple windows issues?
There was a problem hiding this comment.
@ofekby incorrect, all JB windows are under the same process. in vscode its as you say, each window is a process, but not in JB.
| import com.tabnineSelfHosted.binary.requests.userInfo.UserInfoResponse | ||
| import java.util.function.Consumer | ||
|
|
||
| class UserInfoStateSingleton private constructor() : |
There was a problem hiding this comment.
same question regarding this singleton, should it be persisted?
There was a problem hiding this comment.
In general for live data like that I think we shouldn't persist the data, and just have a loading state. Other IDEs can signout while IJ was closed, which will cause a weird startup.
There was a problem hiding this comment.
what will happen when we encounter network issues?
| ENABLED_TOPIC, ChatState.loading() | ||
| ) { | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
why companion object? will the result be different from a static singleton?
There was a problem hiding this comment.
Companion object is basically a static instance. For the implementation of the class I always prefer not implementing statically. In general I don't really like singletons but the initialisation process is a little bit complex so it will do for now.
| notifyListeners() | ||
| } | ||
|
|
||
| fun useState(parent: Disposable, subscription: S) { |
| } | ||
|
|
||
| CapabilitiesStateSingleton.instance.useState { | ||
| updateEnabled() |
There was a problem hiding this comment.
Check for using the current value
| import com.tabnineCommon.chat.actions.AbstractTabnineQuickFixAction | ||
|
|
||
| class TabnineQuickFixAction : AbstractTabnineQuickFixAction() { | ||
| override fun isChatEnabled() = ChatEnabledState.instance.get().enabled |
There was a problem hiding this comment.
Add to the other one prefix
No description provided.