-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Convert actions to Kotlin #482
base: master
Are you sure you want to change the base?
chore: Convert actions to Kotlin #482
Conversation
TelemetryAction.IDE_ACTION | ||
.createActionMessage() | ||
.property("group", null) | ||
.property("action", actionType.name) | ||
.send() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic changed: Success or Failure telemetry, not both (therefore properties added to error too).
But I can revert if both are needed in case of an error 😅
} | ||
|
||
override fun actionPerformed(project: Project, editor: Editor, selectedText: String?) { | ||
if (!selectedText.isNullOrBlank()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed empty to blank
"Staged git diff" to stagedGitDiff, | ||
"New files" to newFilesContent) | ||
.entries.stream() | ||
.filter { !it.value.isNullOrBlank() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed empty to blank
|
||
fun updateState(checkedNode: CheckedTreeNode) { | ||
val fileContent = getNodeFileContent(checkedNode) ?: return | ||
val tokenCount = encodingManager.countTokens(fileContent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do nothing if tokenCount == 0
(for example if an error occured) 😅
.send() | ||
} | ||
} | ||
onRefresh.run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why call onRefresh if event.project == null
? 🤔
Why show a dialog if nothing can be done? 😉
} | ||
|
||
override fun actionPerformed(event: AnActionEvent) { | ||
if (OverlayUtil.showDeleteConversationDialog() == Messages.YES && event.project != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why show a dialog if event.project == null
? 😅
if (OverlayUtil.showDeleteConversationDialog() == Messages.YES && event.project != null) { | |
if (event.project != null && OverlayUtil.showDeleteConversationDialog() == Messages.YES) { |
TelemetryAction.IDE_ACTION.createActionMessage() | ||
.property("action", ActionType.OPEN_CONVERSATION_IN_EDITOR.name) | ||
.send() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Telemetry if nothing happend (e.project == null
or getCurrentConversation() == null
)? 🤔
d00c766
to
ed6f96f
Compare
@carlrobertoh I converted |
I haven't had a chance to review this PR yet, but I'll try to do it sometime early next week. Also, I'll probably do the next release without these refactorings because our test coverage is bad, and I don't want to risk breaking anything. |
Sounds good 👍 |
Speaking of which, please try Llama 3 locally first before releasing 😅 |
Thank you for testing and fixing! Stream was not the problem, |
I followed the template format from their official guide. Even if it's not required, I'd still like to have it included unless it breaks something.
I called it a hack because the way we detect if the server is up and running is through the server logs. I assume they changed their logging structure (from |
Hey, I'm putting this refactoring on pause for a bit until I have run a compatibility comparison between different IDE builds. Most likely, this won't cause any issues, but I just want to be sure, since I'm slowly considering supporting older versions again. |
2c68fde
to
a24a6b6
Compare
a24a6b6
to
9f09aaf
Compare
7683b53
to
6479604
Compare
e57fa37
to
c417cca
Compare
ea9d9ee
to
b4dfde3
Compare
I suggest streamlining all
actionPerformed
:requireNonNull()
or handle null accordingly?