Re: Warning message if clangd not found not displayed ?

Alexander Neundorf <[email protected]> Wed, 05 Oct 2022 23:50:25 +0200
Newsgroups gmane.comp.kde.devel.kwrite
Message-ID <3138343.vfdyTQepKt@unknown0090f5ef9f13> 
On Sonntag, 2. Oktober 2022 17:01:24 CEST you wrote:
> On 2022-10-02 14:23, Alexander Neundorf wrote:
> > On Mittwoch, 28. September 2022 00:39:07 CEST Alexander Neundorf wrote:
> >> Hi,
> >> 
> >> I digged a bit into my clangd problems, and got a little bit further.
> >> If clangd is not installed (it doesn't crash anymore, I think it did
> >> in
> >> older versions, that's good :-) ),
> >> In LSPClientServerManagerImpl::_findServer() it ends up in the branch
> >> "we didn't find the server at all!", where a warning should be
> >> displayed
> >> using showMessage(), but I can't see the warning. "LSP Client" ->
> >> "More
> >> Options" -> "Show Messages" is checked.
> >> Where should the message appear ?
> > 
> > ok, I found it. The message that clangd could not be started is as a
> > warning
> > in the "Output" tab, which I usually never open.
> > I really looked closely at kate to find the message, and I found it
> > only after
> > digging into the sources. I would assume that many users will not see
> > it.
> > 
> > IMO this warning message is so important that the user needs to see it
> > (because it means that "Switch header" does not work, which at least to
> > me is
> > essential in navigating between files).
> > I could change it from Warning to Error, then the tab will by default
> > be
> > opened if the message is issued.
> > This may happen multiple times, e.g. also for each different language.
> > I could add a flag so that it is issued only once as error for every
> > language,
> > and then as warning afterwards ?
> 
> Hi,
> 
> I think we downgraded this to warning as it was super annoying to have
> always the
> output tab there.
> 
> On the other side, you now need to acknowledge the start the first time,
> too, perhaps a first time error would make sense, too.
> 
> But the acknowledge is persistent, over application runs, I think the
> same must be true for errors, if we want that.

how about the attached patch ?
The user will see the error once per kate instance, which makes sense to me.

If you think this is reasonable, I'll create a merge request etc.

Alex
show_as_error_omce.patch (text/x-patch, 2.4 KB) 
diff --git a/addons/lspclient/lspclientservermanager.cpp b/addons/lspclient/lspclientservermanager.cpp
index 5904c8837..2c262cbcd 100644
--- a/addons/lspclient/lspclientservermanager.cpp
+++ b/addons/lspclient/lspclientservermanager.cpp
@@ -749,7 +749,13 @@ private:
                     message += QStringLiteral("\n") + i18n("Please check your PATH for the binary");
                     message += QStringLiteral("\n") + i18n("See also %1 for installation or details", url);
                 }
-                showMessage(message, KTextEditor::Message::Warning);
+                KTextEditor::Message::MessageType messageType = KTextEditor::Message::Warning;
+                if (m_serverNotFoundAlreadyReported.find(QString::fromUtf8("find")+cmdline[0]) == m_serverNotFoundAlreadyReported.end()) {
+                    m_serverNotFoundAlreadyReported.insert(QString::fromUtf8("find")+cmdline[0]);
+                    messageType = KTextEditor::Message::Error;
+                }
+                
+                showMessage(message, messageType);
                 // clear to cut branch below
                 cmdline.clear();
             }
@@ -780,7 +786,13 @@ private:
                     message += QStringLiteral("\n") + i18n("Please check your PATH for the binary");
                     message += QStringLiteral("\n") + i18n("See also %1 for installation or details", url);
                 }
-                showMessage(message, KTextEditor::Message::Warning);
+                
+                KTextEditor::Message::MessageType messageType = KTextEditor::Message::Warning;
+                if (m_serverNotFoundAlreadyReported.find(QString::fromUtf8("start")+cmdline[0]) == m_serverNotFoundAlreadyReported.end()) {
+                    m_serverNotFoundAlreadyReported.insert(QString::fromUtf8("start")+cmdline[0]);
+                    messageType = KTextEditor::Message::Error;
+                }
+                showMessage(message, messageType);
             } else {
                 showMessage(i18n("Started server %2: %1", cmdline.join(QLatin1Char(' ')), serverDescription(server.data())), KTextEditor::Message::Positive);
                 using namespace std::placeholders;
@@ -1122,6 +1134,8 @@ private:
 
         handled = true;
     }
+    
+    std::set<QString> m_serverNotFoundAlreadyReported;
 };
 
 QSharedPointer<LSPClientServerManager> LSPClientServerManager::new_(LSPClientPlugin *plugin)