add functions to access keywords
ClosedPublic

Authored by jpoelen on Jul 27 2018, 11:52 PM.

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.
jpoelen created this revision.Jul 27 2018, 11:52 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJul 27 2018, 11:52 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
jpoelen requested review of this revision.Jul 27 2018, 11:52 PM

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 ↗(On Diff #38629)

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 ↗(On Diff #38629)

Same here:

/**
  * Returns the list of keywords for the keyword list @p name.
  * @since 5.49
  * @see keywordLists()
  */
 QStringList keywordList(const QString& name) const;

No objection from me, assuming you have a use-case for this (auto-completion?).

In general looks good to me, so +1. I would like to have another +1 from @cullmann, @vkrause or @asemke

+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.

asemke accepted this revision.Jul 29 2018, 4:19 PM
This revision is now accepted and ready to land.Jul 29 2018, 4:19 PM

@asemke Are you sure you reference the correct Task? I cannot find anything about keyword lists in your link.

Besides that, my change request with respect to API naming still stands.

@jpoelen Before committing, lets have another review round, could you please provide an updated patch?

This revision was automatically updated to reflect the committed changes.

@asemke Are you sure you reference the correct Task? I cannot find anything about keyword lists in your link.

@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.