VIM-1341 implement gx command #773
Conversation
|
Hi there! Great, this is a long-awaited feature. I've added a couple of comments. |
|
I just saw this. Edit: I'm having trouble finding the comments. I'll check back again in a bit. |
|
why 'gx' open in browser not work? |
|
Hi @GhengS, This feature has not yet been merged with the IdeaVim plugin. |
|
@AlexPl292 I can't find the comments. Would you mind checking the submit review button? I'm guessing because I've made this mistake often with gh's ui. |
| operatorArguments: OperatorArguments, | ||
| ): Boolean { | ||
| val wordUnderCursor = exactWordUnderCursor(editor); | ||
| logger.info("word: $wordUnderCursor") |
There was a problem hiding this comment.
Logging is always great, but we try not to log the raw text from user input. The problem is that this text may have sensitive information like passwords and we don't want to leak it into the log.
I see several cases here, it's better to clean them up.
| ): Boolean { | ||
| val wordUnderCursor = exactWordUnderCursor(editor); | ||
| logger.info("word: $wordUnderCursor") | ||
| if(!isValidUrl(wordUnderCursor)){ |
There was a problem hiding this comment.
I'm thinking about the logic behind links search.
Generally, it looks valid to me, but IJ also has a feature to navigate to issues in Jira or youtrack by issue id. Like // Here is my issue: VIM-123. This, of course, won't be supported by this logic.
I actually found a fancy util in the platform: IssueNavigationConfiguration.getInstance(project).findIssueLinks(commentText). You can feed it a bunch of text and it will return resolved links (Including transformation from VIM-123 to https://youtrack.com/issues/VIM-123).
I think, it will be better to use this util. This, however, will make the code platform dependent, and we'll have two options here: move this GotoUrlAction out from vim-engine or make an abstraction via VimInjector. The second option would require to add a new function to VimInjector like VimApplication.resolveLinks and implement it via the mentioned util in IjVimInjector.
|
Oh, I'm really sorry, my eyes were ignoring the |
|
So I see your comments and have onced over how to address them. I'll put some time into this when I'm not under deadline pressure at my day job. |
| @@ -23,7 +23,7 @@ repositories { | |||
| dependencies { | |||
| compileOnly("com.google.devtools.ksp:symbol-processing-api:1.9.21-1.0.15") | |||
| implementation("org.jetbrains.kotlinx:kotlinx-serialization-json-jvm:$kotlinxSerializationVersion") { | |||
| // kotlin stdlib is provided by IJ, so there is no need to include it into the distribution | |||
| // kotlin stdlib is provided by IJ, so there is no need to include it into the | |||
If anything is not right here please let me know so I can fix it.
I absolutely love this plugin.