From 17267bb15c2baa9bc9d60843a7c0ce037bce7cee Mon Sep 17 00:00:00 2001 From: hjk Date: Fri, 14 Jul 2023 16:29:11 +0200 Subject: [PATCH] Copilot: Move closer to latest settings setup and usage pattern Change-Id: I86983f55cd53a540e2fe1a5b307ebf67f7ea7a98 Reviewed-by: Marcus Tillmanns --- src/plugins/copilot/CMakeLists.txt | 1 - src/plugins/copilot/copilot.qbs | 2 - src/plugins/copilot/copilotclient.cpp | 4 +- src/plugins/copilot/copilotoptionspage.cpp | 117 --------------------- src/plugins/copilot/copilotoptionspage.h | 18 ---- src/plugins/copilot/copilotplugin.cpp | 38 +++---- src/plugins/copilot/copilotplugin.h | 1 - src/plugins/copilot/copilotsettings.cpp | 116 +++++++++++++++++++- src/plugins/copilot/copilotsettings.h | 8 +- 9 files changed, 130 insertions(+), 175 deletions(-) delete mode 100644 src/plugins/copilot/copilotoptionspage.cpp delete mode 100644 src/plugins/copilot/copilotoptionspage.h diff --git a/src/plugins/copilot/CMakeLists.txt b/src/plugins/copilot/CMakeLists.txt index 01129674fce..f50890d0fd2 100644 --- a/src/plugins/copilot/CMakeLists.txt +++ b/src/plugins/copilot/CMakeLists.txt @@ -6,7 +6,6 @@ add_qtc_plugin(Copilot copilotclient.cpp copilotclient.h copilotconstants.h copilothoverhandler.cpp copilothoverhandler.h - copilotoptionspage.cpp copilotoptionspage.h copilotplugin.cpp copilotplugin.h copilotprojectpanel.cpp copilotprojectpanel.h copilotsettings.cpp copilotsettings.h diff --git a/src/plugins/copilot/copilot.qbs b/src/plugins/copilot/copilot.qbs index 714c45543d4..6d2b2100472 100644 --- a/src/plugins/copilot/copilot.qbs +++ b/src/plugins/copilot/copilot.qbs @@ -18,8 +18,6 @@ QtcPlugin { "copilotconstants.h", "copilothoverhandler.cpp", "copilothoverhandler.h", - "copilotoptionspage.cpp", - "copilotoptionspage.h", "copilotplugin.cpp", "copilotplugin.h", "copilotprojectpanel.cpp", diff --git a/src/plugins/copilot/copilotclient.cpp b/src/plugins/copilot/copilotclient.cpp index 061d5d4a459..3f624ca071c 100644 --- a/src/plugins/copilot/copilotclient.cpp +++ b/src/plugins/copilot/copilotclient.cpp @@ -92,7 +92,7 @@ void CopilotClient::openDocument(TextDocument *document) this, [this, document](int position, int charsRemoved, int charsAdded) { Q_UNUSED(charsRemoved) - if (!CopilotSettings::instance().autoComplete()) + if (!settings().autoComplete()) return; auto project = ProjectManager::projectForFile(document->filePath()); @@ -263,7 +263,7 @@ bool CopilotClient::canOpenProject(Project *project) bool CopilotClient::isEnabled(Project *project) { if (!project) - return CopilotSettings::instance().enableCopilot(); + return settings().enableCopilot(); CopilotProjectSettings settings(project); return settings.isEnabled(); diff --git a/src/plugins/copilot/copilotoptionspage.cpp b/src/plugins/copilot/copilotoptionspage.cpp deleted file mode 100644 index 07e21bf91e3..00000000000 --- a/src/plugins/copilot/copilotoptionspage.cpp +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright (C) 2023 The Qt Company Ltd. -// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0 - -#include "copilotoptionspage.h" - -#include "authwidget.h" -#include "copilotconstants.h" -#include "copilotsettings.h" -#include "copilottr.h" - -#include -#include - -#include - -using namespace Utils; -using namespace LanguageClient; - -namespace Copilot { - -class CopilotOptionsPageWidget : public Core::IOptionsPageWidget -{ -public: - CopilotOptionsPageWidget() - { - using namespace Layouting; - - auto warningLabel = new QLabel; - warningLabel->setWordWrap(true); - warningLabel->setTextInteractionFlags(Qt::LinksAccessibleByMouse - | Qt::LinksAccessibleByKeyboard - | Qt::TextSelectableByMouse); - warningLabel->setText(Tr::tr( - "Enabling %1 is subject to your agreement and abidance with your applicable " - "%1 terms. It is your responsibility to know and accept the requirements and " - "parameters of using tools like %1. This may include, but is not limited to, " - "ensuring you have the rights to allow %1 access to your code, as well as " - "understanding any implications of your use of %1 and suggestions produced " - "(like copyright, accuracy, etc.)." ).arg("Copilot")); - - auto authWidget = new AuthWidget(); - - auto helpLabel = new QLabel(); - helpLabel->setTextFormat(Qt::MarkdownText); - helpLabel->setWordWrap(true); - helpLabel->setTextInteractionFlags(Qt::LinksAccessibleByMouse - | Qt::LinksAccessibleByKeyboard - | Qt::TextSelectableByMouse); - helpLabel->setOpenExternalLinks(true); - connect(helpLabel, &QLabel::linkHovered, [](const QString &link) { - QToolTip::showText(QCursor::pos(), link); - }); - - // clang-format off - helpLabel->setText(Tr::tr( - "The Copilot plugin requires node.js and the Copilot neovim plugin. " - "If you install the neovim plugin as described in %1, " - "the plugin will find the agent.js file automatically.\n\n" - "Otherwise you need to specify the path to the %2 " - "file from the Copilot neovim plugin.", - "Markdown text for the copilot instruction label") - .arg("[README.md](https://github.com/github/copilot.vim)") - .arg("[agent.js](https://github.com/github/copilot.vim/tree/release/copilot/dist)")); - - Column { - QString("" + Tr::tr("Note:") + ""), br, - warningLabel, br, - CopilotSettings::instance().enableCopilot, br, - authWidget, br, - CopilotSettings::instance().nodeJsPath, br, - CopilotSettings::instance().distPath, br, - CopilotSettings::instance().autoComplete, br, - helpLabel, br, - st - }.attachTo(this); - // clang-format on - - auto updateAuthWidget = [authWidget]() { - authWidget->updateClient( - FilePath::fromUserInput(CopilotSettings::instance().nodeJsPath.volatileValue()), - FilePath::fromUserInput(CopilotSettings::instance().distPath.volatileValue())); - }; - - connect(CopilotSettings::instance().nodeJsPath.pathChooser(), - &PathChooser::textChanged, - authWidget, - updateAuthWidget); - connect(CopilotSettings::instance().distPath.pathChooser(), - &PathChooser::textChanged, - authWidget, - updateAuthWidget); - updateAuthWidget(); - - setOnApply([] { - CopilotSettings::instance().apply(); - CopilotSettings::instance().writeSettings(); - }); - } -}; - -CopilotOptionsPage::CopilotOptionsPage() -{ - setId(Constants::COPILOT_GENERAL_OPTIONS_ID); - setDisplayName("Copilot"); - setCategory(Constants::COPILOT_GENERAL_OPTIONS_CATEGORY); - setDisplayCategory(Constants::COPILOT_GENERAL_OPTIONS_DISPLAY_CATEGORY); - setCategoryIconPath(":/copilot/images/settingscategory_copilot.png"); - setWidgetCreator([] { return new CopilotOptionsPageWidget; }); -} - -CopilotOptionsPage &CopilotOptionsPage::instance() -{ - static CopilotOptionsPage settingsPage; - return settingsPage; -} - -} // namespace Copilot diff --git a/src/plugins/copilot/copilotoptionspage.h b/src/plugins/copilot/copilotoptionspage.h deleted file mode 100644 index 103e975b634..00000000000 --- a/src/plugins/copilot/copilotoptionspage.h +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (C) 2023 The Qt Company Ltd. -// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0 - -#pragma once - -#include - -namespace Copilot { - -class CopilotOptionsPage : public Core::IOptionsPage -{ -public: - CopilotOptionsPage(); - - static CopilotOptionsPage &instance(); -}; - -} // Copilot diff --git a/src/plugins/copilot/copilotplugin.cpp b/src/plugins/copilot/copilotplugin.cpp index 4ecb1fdf40a..cfc5ab8c83c 100644 --- a/src/plugins/copilot/copilotplugin.cpp +++ b/src/plugins/copilot/copilotplugin.cpp @@ -6,7 +6,6 @@ #include "copilotclient.h" #include "copilotconstants.h" #include "copiloticons.h" -#include "copilotoptionspage.h" #include "copilotprojectpanel.h" #include "copilotsettings.h" #include "copilotsuggestion.h" @@ -34,6 +33,7 @@ namespace Copilot { namespace Internal { enum Direction { Previous, Next }; + void cycleSuggestion(TextEditor::TextEditorWidget *editor, Direction direction) { QTextBlock block = editor->textCursor().block(); @@ -57,14 +57,9 @@ void cycleSuggestion(TextEditor::TextEditorWidget *editor, Direction direction) void CopilotPlugin::initialize() { - CopilotSettings::instance().readSettings(); - restartClient(); - connect(&CopilotSettings::instance(), - &CopilotSettings::applied, - this, - &CopilotPlugin::restartClient); + connect(&settings(), &AspectContainer::applied, this, &CopilotPlugin::restartClient); QAction *requestAction = new QAction(this); requestAction->setText(Tr::tr("Request Copilot Suggestion")); @@ -108,8 +103,8 @@ void CopilotPlugin::initialize() disableAction->setText(Tr::tr("Disable Copilot")); disableAction->setToolTip(Tr::tr("Disable Copilot.")); connect(disableAction, &QAction::triggered, this, [] { - CopilotSettings::instance().enableCopilot.setValue(true); - CopilotSettings::instance().apply(); + settings().enableCopilot.setValue(true); + settings().apply(); }); ActionManager::registerAction(disableAction, Constants::COPILOT_DISABLE); @@ -117,32 +112,31 @@ void CopilotPlugin::initialize() enableAction->setText(Tr::tr("Enable Copilot")); enableAction->setToolTip(Tr::tr("Enable Copilot.")); connect(enableAction, &QAction::triggered, this, [] { - CopilotSettings::instance().enableCopilot.setValue(false); - CopilotSettings::instance().apply(); + settings().enableCopilot.setValue(false); + settings().apply(); }); ActionManager::registerAction(enableAction, Constants::COPILOT_ENABLE); QAction *toggleAction = new QAction(this); toggleAction->setText(Tr::tr("Toggle Copilot")); toggleAction->setCheckable(true); - toggleAction->setChecked(CopilotSettings::instance().enableCopilot.value()); + toggleAction->setChecked(settings().enableCopilot()); toggleAction->setIcon(COPILOT_ICON.icon()); connect(toggleAction, &QAction::toggled, this, [](bool checked) { - CopilotSettings::instance().enableCopilot.setValue(checked); - CopilotSettings::instance().apply(); + settings().enableCopilot.setValue(checked); + settings().apply(); }); ActionManager::registerAction(toggleAction, Constants::COPILOT_TOGGLE); auto updateActions = [toggleAction, requestAction] { - const bool enabled = CopilotSettings::instance().enableCopilot.value(); + const bool enabled = settings().enableCopilot(); toggleAction->setToolTip(enabled ? Tr::tr("Disable Copilot.") : Tr::tr("Enable Copilot.")); toggleAction->setChecked(enabled); requestAction->setEnabled(enabled); }; - connect(&CopilotSettings::instance().enableCopilot, &BaseAspect::changed, - this, updateActions); + connect(&settings().enableCopilot, &BaseAspect::changed, this, updateActions); updateActions(); @@ -157,19 +151,13 @@ void CopilotPlugin::initialize() ProjectPanelFactory::registerFactory(panelFactory); } -void CopilotPlugin::extensionsInitialized() -{ - (void)CopilotOptionsPage::instance(); -} - void CopilotPlugin::restartClient() { LanguageClient::LanguageClientManager::shutdownClient(m_client); - if (!CopilotSettings::instance().nodeJsPath().isExecutableFile()) + if (!settings().nodeJsPath().isExecutableFile()) return; - m_client = new CopilotClient(CopilotSettings::instance().nodeJsPath(), - CopilotSettings::instance().distPath()); + m_client = new CopilotClient(settings().nodeJsPath(), settings().distPath()); } ExtensionSystem::IPlugin::ShutdownFlag CopilotPlugin::aboutToShutdown() diff --git a/src/plugins/copilot/copilotplugin.h b/src/plugins/copilot/copilotplugin.h index 8c1a176a732..3533a17e831 100644 --- a/src/plugins/copilot/copilotplugin.h +++ b/src/plugins/copilot/copilotplugin.h @@ -19,7 +19,6 @@ class CopilotPlugin : public ExtensionSystem::IPlugin public: void initialize() override; - void extensionsInitialized() override; void restartClient(); ShutdownFlag aboutToShutdown() override; diff --git a/src/plugins/copilot/copilotsettings.cpp b/src/plugins/copilot/copilotsettings.cpp index 11e31623da1..089e30448aa 100644 --- a/src/plugins/copilot/copilotsettings.cpp +++ b/src/plugins/copilot/copilotsettings.cpp @@ -2,13 +2,21 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0 #include "copilotsettings.h" + +#include "authwidget.h" #include "copilotconstants.h" #include "copilottr.h" +#include + #include #include #include +#include +#include + +#include using namespace Utils; @@ -23,7 +31,7 @@ static void initEnableAspect(BoolAspect &enableCopilot) enableCopilot.setDefaultValue(false); } -CopilotSettings &CopilotSettings::instance() +CopilotSettings &settings() { static CopilotSettings settings; return settings; @@ -79,6 +87,8 @@ CopilotSettings::CopilotSettings() "position after changes to the document.")); initEnableAspect(enableCopilot); + + readSettings(); } CopilotProjectSettings::CopilotProjectSettings(ProjectExplorer::Project *project, QObject *parent) @@ -106,7 +116,7 @@ void CopilotProjectSettings::setUseGlobalSettings(bool useGlobal) bool CopilotProjectSettings::isEnabled() const { if (useGlobalSettings()) - return CopilotSettings::instance().enableCopilot(); + return settings().enableCopilot(); return enableCopilot(); } @@ -117,7 +127,105 @@ void CopilotProjectSettings::save(ProjectExplorer::Project *project) project->setNamedSettings(Constants::COPILOT_PROJECT_SETTINGS_ID, map); // This triggers a restart of the Copilot language server. - CopilotSettings::instance().apply(); + settings().apply(); } -} // namespace Copilot +// CopilotOptionsPageWidget + +class CopilotOptionsPageWidget : public Core::IOptionsPageWidget +{ +public: + CopilotOptionsPageWidget() + { + using namespace Layouting; + + auto warningLabel = new QLabel; + warningLabel->setWordWrap(true); + warningLabel->setTextInteractionFlags(Qt::LinksAccessibleByMouse + | Qt::LinksAccessibleByKeyboard + | Qt::TextSelectableByMouse); + warningLabel->setText(Tr::tr( + "Enabling %1 is subject to your agreement and abidance with your applicable " + "%1 terms. It is your responsibility to know and accept the requirements and " + "parameters of using tools like %1. This may include, but is not limited to, " + "ensuring you have the rights to allow %1 access to your code, as well as " + "understanding any implications of your use of %1 and suggestions produced " + "(like copyright, accuracy, etc.)." ).arg("Copilot")); + + auto authWidget = new AuthWidget(); + + auto helpLabel = new QLabel(); + helpLabel->setTextFormat(Qt::MarkdownText); + helpLabel->setWordWrap(true); + helpLabel->setTextInteractionFlags(Qt::LinksAccessibleByMouse + | Qt::LinksAccessibleByKeyboard + | Qt::TextSelectableByMouse); + helpLabel->setOpenExternalLinks(true); + connect(helpLabel, &QLabel::linkHovered, [](const QString &link) { + QToolTip::showText(QCursor::pos(), link); + }); + + // clang-format off + helpLabel->setText(Tr::tr( + "The Copilot plugin requires node.js and the Copilot neovim plugin. " + "If you install the neovim plugin as described in %1, " + "the plugin will find the agent.js file automatically.\n\n" + "Otherwise you need to specify the path to the %2 " + "file from the Copilot neovim plugin.", + "Markdown text for the copilot instruction label") + .arg("[README.md](https://github.com/github/copilot.vim)") + .arg("[agent.js](https://github.com/github/copilot.vim/tree/release/copilot/dist)")); + + Column { + QString("" + Tr::tr("Note:") + ""), br, + warningLabel, br, + settings().enableCopilot, br, + authWidget, br, + settings().nodeJsPath, br, + settings().distPath, br, + settings().autoComplete, br, + helpLabel, br, + st + }.attachTo(this); + // clang-format on + + auto updateAuthWidget = [authWidget]() { + authWidget->updateClient( + FilePath::fromUserInput(settings().nodeJsPath.volatileValue()), + FilePath::fromUserInput(settings().distPath.volatileValue())); + }; + + connect(settings().nodeJsPath.pathChooser(), + &PathChooser::textChanged, + authWidget, + updateAuthWidget); + connect(settings().distPath.pathChooser(), + &PathChooser::textChanged, + authWidget, + updateAuthWidget); + updateAuthWidget(); + + setOnApply([] { + settings().apply(); + settings().writeSettings(); + }); + } +}; + +class CopilotSettingsPage : public Core::IOptionsPage +{ +public: + CopilotSettingsPage() + { + setId(Constants::COPILOT_GENERAL_OPTIONS_ID); + setDisplayName("Copilot"); + setCategory(Constants::COPILOT_GENERAL_OPTIONS_CATEGORY); + setDisplayCategory(Constants::COPILOT_GENERAL_OPTIONS_DISPLAY_CATEGORY); + setCategoryIconPath(":/copilot/images/settingscategory_copilot.png"); + setWidgetCreator([] { return new CopilotOptionsPageWidget; }); + } +}; + +const CopilotSettingsPage settingsPage; + +} // Copilot diff --git a/src/plugins/copilot/copilotsettings.h b/src/plugins/copilot/copilotsettings.h index cec44c43fed..2f66f48ffba 100644 --- a/src/plugins/copilot/copilotsettings.h +++ b/src/plugins/copilot/copilotsettings.h @@ -5,9 +5,7 @@ #include -namespace ProjectExplorer { -class Project; -} +namespace ProjectExplorer { class Project; } namespace Copilot { @@ -16,14 +14,14 @@ class CopilotSettings : public Utils::AspectContainer public: CopilotSettings(); - static CopilotSettings &instance(); - Utils::FilePathAspect nodeJsPath{this}; Utils::FilePathAspect distPath{this}; Utils::BoolAspect autoComplete{this}; Utils::BoolAspect enableCopilot{this}; }; +CopilotSettings &settings(); + class CopilotProjectSettings : public Utils::AspectContainer { public: