Details
- Reviewers
dhaumann asemke - Group Reviewers
Framework: Syntax Highlighting - Commits
- R216:9e3f07de97ea: add functions to access keywords
Diff Detail
- Repository
- R216 Syntax Highlighting
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
In general looks good to me, so +1. I would like to have another +1 from @cullmann, @vkrause or @asemke
What I wonder is whether you really need the keyword lists from the Definition, or whether you want the currently active keyword lists from the current State / Context. I can see that both is useful.
src/lib/definition.h | ||
---|---|---|
175–176 | Could we change this to: /** * Returns the names of the keyword lists of this Definition. * @since 5.49 * @see keywordList() */ QStringList keywordLists() const; | |
177–178 | Same here: /** * Returns the list of keywords for the keyword list @p name. * @since 5.49 * @see keywordLists() */ QStringList keywordList(const QString& name) const; |
+1
What I wonder is whether you really need the keyword lists from the Definition, or whether you want the currently active keyword lists from the current State / Context. I can see that both is useful.
As just replied to the mailing list, both would be useful. But let's do this maybe step by step. With this change T4760 will become possible. After this I'll have a look at how to simplify the completion logic in Cantor.
@dhaumann yes, I'm sure this is the correct task. That task is about the refactoring of the "login-mechanism". The "login" should be only done if needed or if possible. At the moment the login to some systems has to _always_ be done because the list of keywords is fetched dynamically during the runtime - this makes T4760 impossible. The keyworkds are needed to properly highlight the content. After the move to KSyntaxHighlighting we can use static lists of the keywords provided by this library and there is no need to login anymore until there is a real need for this, i.e. the user starts calculations.
@asemke Ok, thanks. It's available for 5.49, which is scheduled for released for Saturda 11th of August.