Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Introduce a type for CodebaseName #996

Merged
merged 10 commits into from
Mar 7, 2024
Merged

Introduce a type for CodebaseName #996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0e385f0
Introduce a type for CodebaseName
mkondratek Mar 6, 2024
3c26d44
Fixes
mkondratek Mar 7, 2024
275e6a4
lowercase cloneUrl in convertGitCloneURLToCodebaseNameOrError
mkondratek Mar 7, 2024
5e3e2bd
Add a test
mkondratek Mar 7, 2024
84fc070
Enhance test
mkondratek Mar 7, 2024
b84df65
Fixes
mkondratek Mar 7, 2024
476ca23
Migrate chat's context too
mkondratek Mar 7, 2024
2068473
Fix renderer
mkondratek Mar 7, 2024
419f13c
Fix ConvertUtilTest
mkondratek Mar 7, 2024
0480b77
distinctBy
mkondratek Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/main/java/com/sourcegraph/vcs/ConvertUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package com.sourcegraph.vcs

import java.net.URI

fun convertGitCloneURLToCodebaseNameOrError(cloneURL: String): String {
data class CodebaseName(val value: String)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the value always some kind of the url?
If yes, maybe we can make the name more descriptive?
If no, maybe it warrants a comment what value we can expect there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CodebaseName is the description here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codebase name does not suggest url-like-string at all to me.
And it's not defined anywhere what it really means.
Anyway, it's just my point of view, there is nothing wrong with the code per se.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codebase name does not suggest url-like-string at all to me.

tbh I'd say that's a good sign - it will be harder to confuse it with URL 😅


fun convertGitCloneURLToCodebaseNameOrError(theCloneURL: String): CodebaseName {
val cloneURL = theCloneURL.lowercase()

// Handle common Git SSH URL format
val sshUrlRegexMatchResult = Regex("""^[\w-]+@([^:]+):([\w-]+)/([\w-.]+)$""").find(cloneURL)
if (sshUrlRegexMatchResult != null) {
val (host, owner, repo) = sshUrlRegexMatchResult.destructured
return "$host/$owner/${repo.replace(".git$".toRegex(), "")}"
return CodebaseName("$host/$owner/${repo.replace(".git$".toRegex(), "")}")
}

var uri = URI(cloneURL)
Expand All @@ -18,29 +21,29 @@ fun convertGitCloneURLToCodebaseNameOrError(cloneURL: String): String {

// Handle Azure DevOps URLs
if (uri.host?.contains("dev.azure") == true && !uri.path.isNullOrEmpty()) {
return "${uri.host}${uri.path.replace("/_git", "")}"
return CodebaseName("${uri.host}${uri.path.replace("/_git", "")}")
}

// Handle GitHub URLs
if (uri.scheme?.startsWith("github") == true) {
return "github.com/${uri.schemeSpecificPart.replace(".git", "")}"
return CodebaseName("github.com/${uri.schemeSpecificPart.replace(".git", "")}")
}

// Handle GitLab URLs
if (uri.scheme?.startsWith("gitlab") == true) {
return "gitlab.com/${uri.schemeSpecificPart.replace(".git", "")}"
return CodebaseName("gitlab.com/${uri.schemeSpecificPart.replace(".git", "")}")
}

// Handle HTTPS URLs
if (uri.scheme?.startsWith("http") == true &&
!uri.host.isNullOrEmpty() &&
!uri.path.isNullOrEmpty()) {
return "${uri.host}${uri.path?.replace(".git", "")}"
return CodebaseName("${uri.host}${uri.path?.replace(".git", "")}")
}

// Generic URL
if (uri.host != null && !uri.path.isNullOrEmpty()) {
return "${uri.host}${uri.path.replace(".git", "")}"
return CodebaseName("${uri.host}${uri.path.replace(".git", "")}")
}

throw Exception("Cody could not extract repo name from clone URL $cloneURL")
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/sourcegraph/vcs/RepoUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private static String doReplacements(

if (vcsType == VCSType.GIT && repository != null) {
String cloneURL = GitUtil.getRemoteRepoUrl((GitRepository) repository, project);
return convertGitCloneURLToCodebaseNameOrError(cloneURL);
return convertGitCloneURLToCodebaseNameOrError(cloneURL).getValue();
}

if (vcsType == VCSType.PERFORCE) {
Expand Down
37 changes: 37 additions & 0 deletions src/main/kotlin/com/sourcegraph/cody/config/SettingsMigration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ import com.sourcegraph.cody.CodyToolWindowFactory
import com.sourcegraph.cody.api.SourcegraphApiRequestExecutor
import com.sourcegraph.cody.api.SourcegraphApiRequests
import com.sourcegraph.cody.history.HistoryService
import com.sourcegraph.cody.history.state.ChatState
import com.sourcegraph.cody.history.state.EnhancedContextState
import com.sourcegraph.cody.history.state.LLMState
import com.sourcegraph.cody.initialization.Activity
import com.sourcegraph.config.AccessTokenStorage
import com.sourcegraph.config.CodyApplicationService
import com.sourcegraph.config.CodyProjectService
import com.sourcegraph.config.ConfigUtil
import com.sourcegraph.config.UserLevelConfig
import com.sourcegraph.vcs.convertGitCloneURLToCodebaseNameOrError
import java.util.concurrent.CompletableFuture

class SettingsMigration : Activity {
Expand All @@ -35,6 +38,9 @@ class SettingsMigration : Activity {
migrateAccounts(project)
}
RunOnceUtil.runOnceForProject(project, "CodyHistoryLlmMigration") { migrateLlms(project) }
RunOnceUtil.runOnceForProject(project, "CodyConvertUrlToCodebaseName") {
migrateUrlsToCodebaseNames(project)
}
RunOnceUtil.runOnceForApp("CodyApplicationSettingsMigration") { migrateApplicationSettings() }
RunOnceUtil.runOnceForApp("ToggleCodyToolWindowAfterMigration") {
toggleCodyToolbarWindow(project)
Expand Down Expand Up @@ -105,6 +111,19 @@ class SettingsMigration : Activity {
}
}

private fun migrateUrlsToCodebaseNames(project: Project) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for that.
Especially considering those cases:

  • previously added codebase name without http://
  • previously added codebase name with upercase address

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

val enhancedContextState =
HistoryService.getInstance(project).state.defaultEnhancedContext ?: return

migrateUrlsToCodebaseNames(enhancedContextState)

HistoryService.getInstance(project)
.state
.chats
.mapNotNull(ChatState::enhancedContext)
.forEach(Companion::migrateUrlsToCodebaseNames)
}

private val modelToProviderAndTitle =
mapOf(
"Claude 2 by Anthropic" to Triple("anthropic/claude-2", "Anthropic", "claude 2"),
Expand Down Expand Up @@ -391,5 +410,23 @@ class SettingsMigration : Activity {

companion object {
private val LOG = logger<SettingsMigration>()

fun migrateUrlsToCodebaseNames(enhancedContextState: EnhancedContextState) {
val remoteRepositories =
enhancedContextState.remoteRepositories
.onEach { remoteRepositoryState ->
runCatching {
remoteRepositoryState.remoteUrl?.let { remoteUrl ->
convertGitCloneURLToCodebaseNameOrError(remoteUrl)
}
}
.getOrNull()
?.let { remoteRepositoryState.codebaseName = it.value }
}
.filter { it.codebaseName != null }
.distinctBy { it.codebaseName }
.toMutableList()
enhancedContextState.remoteRepositories = remoteRepositories
}
}
}
5 changes: 3 additions & 2 deletions src/main/kotlin/com/sourcegraph/cody/context/RemoteRepoUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import com.intellij.openapi.project.Project
import com.sourcegraph.cody.agent.CodyAgentService
import com.sourcegraph.cody.agent.protocol.GetRepoIdsParam
import com.sourcegraph.cody.agent.protocol.Repo
import com.sourcegraph.vcs.CodebaseName
import java.util.concurrent.CompletableFuture

object RemoteRepoUtils {
fun getRepository(project: Project, codebaseName: String): CompletableFuture<Repo?> {
fun getRepository(project: Project, codebaseName: CodebaseName): CompletableFuture<Repo?> {
val result = CompletableFuture<Repo?>()
CodyAgentService.withAgent(project) { agent ->
try {
val repos = agent.server.getRepoIds(GetRepoIdsParam(listOf(codebaseName), 1)).get()
val repos = agent.server.getRepoIds(GetRepoIdsParam(listOf(codebaseName.value), 1)).get()
result.complete(repos?.repos?.firstOrNull())
} catch (e: Exception) {
result.complete(null)
Expand Down
23 changes: 12 additions & 11 deletions src/main/kotlin/com/sourcegraph/cody/context/ui/AddRepositoryDialog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.intellij.util.Alarm
import com.sourcegraph.cody.config.DialogValidationUtils
import com.sourcegraph.cody.context.RemoteRepoUtils
import com.sourcegraph.common.CodyBundle
import com.sourcegraph.vcs.CodebaseName
import com.sourcegraph.vcs.convertGitCloneURLToCodebaseNameOrError
import java.awt.GridBagConstraints
import java.awt.GridBagLayout
Expand All @@ -23,7 +24,7 @@ import org.jetbrains.annotations.NotNull
class AddRepositoryDialog(
private val project: Project,
private val remoteContextNode: ContextTreeRemoteRootNode,
private val addAction: (String) -> Unit
private val addAction: (CodebaseName) -> Unit
) : DialogWrapper(project) {

private val repoUrlInputField = TextFieldWithAutoCompletion.create(project, listOf(), false, null)
Expand All @@ -36,20 +37,20 @@ class AddRepositoryDialog(
}

override fun doValidateAll(): List<ValidationInfo> {
val text = repoUrlInputField.text.lowercase()

fun validateNonEmpty() =
DialogValidationUtils.custom(
repoUrlInputField,
CodyBundle.getString("context-panel.add-repo-dialog.error-empty-url")) {
repoUrlInputField.text.isNotBlank()
text.isNotBlank()
}

fun validateValidUrl() =
DialogValidationUtils.custom(
repoUrlInputField,
CodyBundle.getString("context-panel.add-repo-dialog.error-invalid-url")) {
val url =
if (repoUrlInputField.text.startsWith("http")) repoUrlInputField.text
else "http://" + repoUrlInputField.text
val url = if (text.startsWith("http")) text else "http://$text"
runCatching { URL(url) }.isSuccess
}

Expand All @@ -58,8 +59,7 @@ class AddRepositoryDialog(
repoUrlInputField,
CodyBundle.getString("context-panel.add-repo-dialog.error-no-repo")) {
val codebaseName =
runCatching { convertGitCloneURLToCodebaseNameOrError(repoUrlInputField.text) }
.getOrNull()
runCatching { convertGitCloneURLToCodebaseNameOrError(text) }.getOrNull()
codebaseName ?: return@custom false
val repo =
RemoteRepoUtils.getRepository(project, codebaseName)
Expand All @@ -73,13 +73,12 @@ class AddRepositoryDialog(
repoUrlInputField,
CodyBundle.getString("context-panel.add-repo-dialog.error-repo-already-added")) {
val codebaseName =
runCatching { convertGitCloneURLToCodebaseNameOrError(repoUrlInputField.text) }
.getOrNull()
runCatching { convertGitCloneURLToCodebaseNameOrError(text) }.getOrNull()
remoteContextNode
.children()
.toList()
.filterIsInstance<ContextTreeRemoteRepoNode>()
.none { it.codebaseName == codebaseName?.lowercase() }
.none { it.codebaseName == codebaseName }
}

return listOfNotNull(
Expand All @@ -94,7 +93,9 @@ class AddRepositoryDialog(
}

override fun doOKAction() {
addAction(repoUrlInputField.text.lowercase())
runCatching { convertGitCloneURLToCodebaseNameOrError(repoUrlInputField.text.lowercase()) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move that conversion to lowercase inside convertGitCloneURLToCodebaseNameOrError?
That way we will never miss it... as we do now in case of the migration?

.getOrNull()
?.let { codebaseName -> addAction(codebaseName) }
close(OK_EXIT_CODE, true)
}

Expand Down
5 changes: 3 additions & 2 deletions src/main/kotlin/com/sourcegraph/cody/context/ui/ContextRepositoriesCheckboxRenderer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ class ContextRepositoriesCheckboxRenderer : CheckboxTree.CheckboxTreeCellRendere

when (node) {
is ContextTreeRemoteRepoNode -> {
val repoName = node.codebaseName.split(File.separator).lastOrNull() ?: node.codebaseName
val repoName =
node.codebaseName.value.split(File.separator).lastOrNull() ?: node.codebaseName.value
textRenderer.appendHTML(
"<b>${repoName}</b> <i ${style}>${node.codebaseName}</i>",
"<b>${repoName}</b> <i ${style}>${node.codebaseName.value}</i>",
SimpleTextAttributes.REGULAR_ATTRIBUTES)
}
is ContextTreeLocalRepoNode -> {
Expand Down
5 changes: 3 additions & 2 deletions src/main/kotlin/com/sourcegraph/cody/context/ui/ContextTreeNode.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.sourcegraph.cody.context.ui

import com.intellij.openapi.project.Project
import com.intellij.ui.CheckedTreeNode
import com.sourcegraph.vcs.CodebaseName
import java.util.concurrent.atomic.AtomicBoolean

open class ContextTreeNode<T>(value: T, private val onSetChecked: (Boolean) -> Unit = {}) :
Expand All @@ -17,8 +18,8 @@ class ContextTreeRootNode(val text: String, onSetChecked: (Boolean) -> Unit) :

class ContextTreeRemoteRootNode(val text: String) : ContextTreeNode<String>(text)

class ContextTreeRemoteRepoNode(val codebaseName: String, onSetChecked: (Boolean) -> Unit) :
ContextTreeNode<String>(codebaseName, onSetChecked)
class ContextTreeRemoteRepoNode(val codebaseName: CodebaseName, onSetChecked: (Boolean) -> Unit) :
ContextTreeNode<CodebaseName>(codebaseName, onSetChecked)

open class ContextTreeLocalNode<T>(value: T, private val isEnhancedContextEnabled: AtomicBoolean) :
ContextTreeNode<T>(value) {
Expand Down
34 changes: 21 additions & 13 deletions src/main/kotlin/com/sourcegraph/cody/context/ui/EnhancedContextPanel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.sourcegraph.cody.history.HistoryService
import com.sourcegraph.cody.history.state.EnhancedContextState
import com.sourcegraph.cody.history.state.RemoteRepositoryState
import com.sourcegraph.common.CodyBundle
import com.sourcegraph.vcs.CodebaseName
import com.sourcegraph.vcs.convertGitCloneURLToCodebaseNameOrError
import java.awt.Dimension
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -84,7 +85,9 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
if (!isDotComAccount()) {
if (contextState != null) {
contextState.remoteRepositories.forEach { repo ->
repo.remoteUrl?.let { remoteUrl -> addRemoteRepository(remoteUrl, repo.isEnabled) }
repo.codebaseName?.let { codebaseName ->
addRemoteRepository(CodebaseName(codebaseName), repo.isEnabled)
}
}
} else {
CodyAgentCodebase.getInstance(project).getUrl().thenApply { repoUrl ->
Expand All @@ -93,7 +96,9 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
.completeOnTimeout(null, 15, TimeUnit.SECONDS)
.thenApply { repo ->
if (repo != null) {
ApplicationManager.getApplication().invokeLater { addRemoteRepository(repoUrl) }
ApplicationManager.getApplication().invokeLater {
addRemoteRepository(codebaseName)
}
}
}
}
Expand Down Expand Up @@ -121,15 +126,16 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
private fun isDotComAccount() =
CodyAuthenticationManager.instance.getActiveAccount(project)?.isDotcomAccount() ?: false

private fun getRepoByUrlAndRun(codebaseName: String, consumer: Consumer<Repo>) {
private fun getRepoByUrlAndRun(codebaseName: CodebaseName, consumer: Consumer<Repo>) {
RemoteRepoUtils.getRepository(project, codebaseName).thenApply {
it?.let { repo -> consumer.accept(repo) }
}
}

private fun enableRemote(codebaseName: String) {
private fun enableRemote(codebaseName: CodebaseName) {
updateContextState { contextState ->
contextState.remoteRepositories.find { it.remoteUrl == codebaseName }?.isEnabled = true
contextState.remoteRepositories.find { it.codebaseName == codebaseName.value }?.isEnabled =
true
}
getRepoByUrlAndRun(codebaseName) { repo ->
chatSession.sendWebviewMessage(
Expand All @@ -139,9 +145,10 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
}

@RequiresEdt
private fun disableRemote(codebaseName: String) {
private fun disableRemote(codebaseName: CodebaseName) {
updateContextState { contextState ->
contextState.remoteRepositories.find { it.remoteUrl == codebaseName }?.isEnabled = false
contextState.remoteRepositories.find { it.codebaseName == codebaseName.value }?.isEnabled =
false
}
getRepoByUrlAndRun(codebaseName) { repo ->
chatSession.sendWebviewMessage(
Expand All @@ -152,7 +159,7 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
@RequiresEdt
private fun removeRemoteRepository(node: ContextTreeRemoteRepoNode) {
updateContextState { contextState ->
contextState.remoteRepositories.removeIf { it.remoteUrl == node.codebaseName }
contextState.remoteRepositories.removeIf { it.codebaseName == node.codebaseName.value }
}
remoteContextNode.remove(node)
if (enhancedContextNode.children().toList().contains(remoteContextNode) &&
Expand All @@ -164,14 +171,13 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
}

@RequiresEdt
private fun addRemoteRepository(repoUrl: String, isCheckedInitially: Boolean = true) {
val codebaseName = convertGitCloneURLToCodebaseNameOrError(repoUrl)
private fun addRemoteRepository(codebaseName: CodebaseName, isCheckedInitially: Boolean = true) {

updateContextState { contextState ->
val repositories = contextState.remoteRepositories
val existingRepo = repositories.find { it.remoteUrl == codebaseName }
val existingRepo = repositories.find { it.codebaseName == codebaseName.value }
val modifiedRepo = existingRepo ?: RemoteRepositoryState()
modifiedRepo.remoteUrl = codebaseName
modifiedRepo.codebaseName = codebaseName.value
modifiedRepo.isEnabled = isCheckedInitially
if (existingRepo == null) repositories.add(modifiedRepo)
}
Expand Down Expand Up @@ -224,7 +230,9 @@ class EnhancedContextPanel(private val project: Project, private val chatSession
toolbarDecorator.setAddActionName(
CodyBundle.getString("context-panel.button.add-remote-repo"))
toolbarDecorator.setAddAction {
AddRepositoryDialog(project, remoteContextNode) { repoUrl -> addRemoteRepository(repoUrl) }
AddRepositoryDialog(project, remoteContextNode) { codebaseName ->
addRemoteRepository(codebaseName)
}
.show()
expandAllNodes()
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/kotlin/com/sourcegraph/cody/history/state/RemoteRepositoryState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ class RemoteRepositoryState : BaseState() {
@get:OptionTag(tag = "isEnabled", nameAttribute = "") var isEnabled: Boolean by property(true)

@get:OptionTag(tag = "remoteUrl", nameAttribute = "") var remoteUrl: String? by string()

@get:OptionTag(tag = "codebaseName", nameAttribute = "") var codebaseName: String? by string()
}
4 changes: 2 additions & 2 deletions src/main/resources/CodyBundle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ context-panel.tree.root=repositories-root
context-panel.tree.node-chat-context=Context for this Chat
context-panel.tree.node-local-project=Local Project
context-panel.tree.node-remote-repos=Remote Repo(s)
context-panel.add-repo-dialog.title=Add Remote Repo
context-panel.add-repo-dialog.title=Add Remote Repository
context-panel.add-repo-dialog.error-empty-url=Remote repository URL cannot be empty
context-panel.add-repo-dialog.error-invalid-url=Remote repository URL must be valid
context-panel.add-repo-dialog.error-no-repo=Remote repository not found on the server
context-panel.add-repo-dialog.error-repo-already-added=Remote repository has been added already
context-panel.add-repo-dialog.url-input-label=<html><b>Repo URL</b><html>
context-panel.add-repo-dialog.url-input-label=<html><b>Repository URL</b><html>
context-panel.add-repo-dialog.url-input-help=<html><span>Example: <b>github.com/sourcegraph/cody</b><br>Only repos indexed on customer instance are valid.<br><br>Contact your Sourcegraph admin to add missing repo.<span><html>
messages-panel.welcome-text=Hello! I'm Cody. I can write code and answer questions for you. See [Cody documentation](https://sourcegraph.com/docs/cody) for help and tips.
chat-session.error-title=Error while processing the message
Expand Down
Loading
Loading