[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwrite-devel
Subject:    Re: Warning message if clangd not found not displayed ?
From:       Alexander Neundorf <neundorf () kde ! org>
Date:       2022-10-05 21:50:25
Message-ID: 3138343.vfdyTQepKt () unknown0090f5ef9f13
[Download RAW message or body]

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" (show_as_error_omce.patch)]

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)



[prev in list] [next in list] [prev in thread] [next in thread]