Skip to content

Commit

Permalink
Fix LT reinitialization when running quick fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
valentjn committed Aug 21, 2021
1 parent 833ab33 commit bcbad28
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 77 deletions.
9 changes: 9 additions & 0 deletions changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
<action type="add" issue="vscode-ltex#391">
Add support for the `main` option of the babel package (LaTeX)
</action>
<action type="add">
Add support for [`ltex.dictionary`](https://valentjn.github.io/vscode-ltex/docs/settings.html#ltexdictionary) when using a LanguageTool HTTP server
</action>
<action type="update" issue="vscode-ltex#390">
Handle disabled rules ourselves to prevent reinitialization of LanguageTool when running the `Disable rule` quick fix
</action>
<action type="fix" issue="vscode-ltex#390">
Fix LanguageTool reinitialized when running the `Add '...' to dictionary` quick fix
</action>
<action type="fix">
Fix used i18n keys removed
</action>
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>org.bsplines</groupId>
<artifactId>ltexls</artifactId>
<version>13.0.1-alpha.1.develop</version>
<version>13.1.0-alpha.1.develop</version>
<name>${project.groupId}:${project.artifactId}</name>
<description>LTeX Language Server (LTeX LS): LSP language server for LanguageTool with support for LaTeX, Markdown, and others</description>
<url>https://github.com/valentjn/ltex-ls</url>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import org.bsplines.ltexls.tools.I18n
import org.bsplines.ltexls.tools.Logging
import org.languagetool.markup.AnnotatedText
import org.languagetool.markup.TextPart
import org.languagetool.rules.RuleMatch
import java.io.IOException
import java.io.UnsupportedEncodingException
import java.net.MalformedURLException
Expand All @@ -34,9 +33,8 @@ class LanguageToolHttpInterface(
uriString: String,
private val languageShortCode: String,
private val motherTongueShortCode: String,
) : LanguageToolInterface {
private val enabledRuleIds: MutableList<String> = ArrayList()
private val disabledRuleIds: MutableList<String> = ArrayList()
) : LanguageToolInterface() {
private val enabledRules: MutableList<String> = ArrayList()
private val httpClient: HttpClient = HttpClient.newHttpClient()
private val uri: URI?

Expand All @@ -61,7 +59,9 @@ class LanguageToolHttpInterface(
return (this.uri != null)
}

override fun check(annotatedTextFragment: AnnotatedTextFragment): List<LanguageToolRuleMatch> {
override fun checkInternal(
annotatedTextFragment: AnnotatedTextFragment,
): List<LanguageToolRuleMatch> {
if (!isInitialized()) return emptyList()

val requestBody: String = createRequestBody(annotatedTextFragment) ?: return emptyList()
Expand Down Expand Up @@ -92,20 +92,9 @@ class LanguageToolHttpInterface(
val result = ArrayList<LanguageToolRuleMatch>()

for (jsonElement: JsonElement in jsonMatches) {
val jsonMatch: JsonObject = jsonElement.asJsonObject
val ruleId: String = jsonMatch.get("rule").asJsonObject.get("id").asString
val sentence: String = jsonMatch.get("sentence").asString
val fromPos: Int = jsonMatch.get("offset").asInt
val toPos: Int = fromPos + jsonMatch.get("length").asInt
val message: String = jsonMatch.get("message").asString
val suggestedReplacements = ArrayList<String>()

for (replacement: JsonElement in jsonMatch.get("replacements").asJsonArray) {
suggestedReplacements.add(replacement.asJsonObject.get("value").asString)
}

result.add(LanguageToolRuleMatch.fromLanguageTool(ruleId, sentence, fromPos, toPos, message,
suggestedReplacements, RuleMatch.Type.Hint, annotatedTextFragment))
result.add(
LanguageToolRuleMatch.fromLanguageTool(jsonElement.asJsonObject, annotatedTextFragment)
)
}

return result
Expand All @@ -127,12 +116,8 @@ class LanguageToolHttpInterface(
requestEntries["motherTongue"] = this.motherTongueShortCode
}

if (this.enabledRuleIds.isNotEmpty()) {
requestEntries["enabledRules"] = this.enabledRuleIds.joinToString(",")
}

if (this.disabledRuleIds.isNotEmpty()) {
requestEntries["disabledRules"] = this.disabledRuleIds.joinToString(",")
if (this.enabledRules.isNotEmpty()) {
requestEntries["enabledRules"] = this.enabledRules.joinToString(",")
}

val builder = StringBuilder()
Expand Down Expand Up @@ -169,13 +154,7 @@ class LanguageToolHttpInterface(
}

override fun enableRules(ruleIds: Set<String>) {
this.enabledRuleIds.addAll(ruleIds)
this.disabledRuleIds.removeAll(ruleIds)
}

override fun disableRules(ruleIds: Set<String>) {
this.enabledRuleIds.removeAll(ruleIds)
this.disabledRuleIds.addAll(ruleIds)
this.enabledRules.addAll(ruleIds)
}

override fun enableEasterEgg() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,45 @@ package org.bsplines.ltexls.languagetool

import org.bsplines.ltexls.parsing.AnnotatedTextFragment

interface LanguageToolInterface {
fun isInitialized(): Boolean
fun check(annotatedTextFragment: AnnotatedTextFragment): List<LanguageToolRuleMatch>
fun activateDefaultFalseFriendRules()
fun activateLanguageModelRules(languageModelRulesDirectory: String)
fun activateNeuralNetworkRules(neuralNetworkRulesDirectory: String)
fun activateWord2VecModelRules(word2vecRulesDirectory: String)
fun enableRules(ruleIds: Set<String>)
fun disableRules(ruleIds: Set<String>)
fun enableEasterEgg()
abstract class LanguageToolInterface {
var dictionary: Set<String> = emptySet()
var disabledRules: Set<String> = emptySet()

fun check(annotatedTextFragment: AnnotatedTextFragment): List<LanguageToolRuleMatch> {
val matches = ArrayList<LanguageToolRuleMatch>()

for (match: LanguageToolRuleMatch in checkInternal(annotatedTextFragment)) {
if (checkMatchValidity(annotatedTextFragment, match)) matches.add(match)
}

return matches
}

protected fun checkMatchValidity(
annotatedTextFragment: AnnotatedTextFragment,
match: LanguageToolRuleMatch,
): Boolean {
return (
(
!match.isUnknownWordRule()
|| !this.dictionary.contains(
annotatedTextFragment.getSubstringOfPlainText(match.fromPos, match.toPos)
)
)
&& !this.disabledRules.contains(match.ruleId)
)
}

abstract fun isInitialized(): Boolean

protected abstract fun checkInternal(
annotatedTextFragment: AnnotatedTextFragment,
): List<LanguageToolRuleMatch>

abstract fun activateDefaultFalseFriendRules()
abstract fun activateLanguageModelRules(languageModelRulesDirectory: String)
abstract fun activateNeuralNetworkRules(neuralNetworkRulesDirectory: String)
abstract fun activateWord2VecModelRules(word2vecRulesDirectory: String)
abstract fun enableRules(ruleIds: Set<String>)
abstract fun enableEasterEgg()
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class LanguageToolJavaInterface(
motherTongueShortCode: String,
sentenceCacheSize: Long,
dictionary: Set<String>,
) : LanguageToolInterface {
private val dictionary: Set<String> = dictionary.toSet()
) : LanguageToolInterface() {
private val resultCache =
ResultCache(sentenceCacheSize, RESULT_CACHE_EXPIRE_AFTER_MINUTES, TimeUnit.MINUTES)
private val languageTool: JLanguageTool?
Expand All @@ -61,7 +60,9 @@ class LanguageToolJavaInterface(
}

@Suppress("INACCESSIBLE_TYPE")
override fun check(annotatedTextFragment: AnnotatedTextFragment): List<LanguageToolRuleMatch> {
override fun checkInternal(
annotatedTextFragment: AnnotatedTextFragment,
): List<LanguageToolRuleMatch> {
val languageTool: JLanguageTool? = this.languageTool

if (languageTool == null) {
Expand Down Expand Up @@ -123,16 +124,7 @@ class LanguageToolJavaInterface(
val result = ArrayList<LanguageToolRuleMatch>()

for (match: RuleMatch in matches) {
val languageToolRuleMatch = LanguageToolRuleMatch.fromLanguageTool(
match, annotatedTextFragment)

if (languageToolRuleMatch.isUnknownWordRule()
&& this.dictionary.contains(annotatedTextFragment.getSubstringOfPlainText(
languageToolRuleMatch.fromPos, languageToolRuleMatch.toPos))) {
continue
}

result.add(languageToolRuleMatch)
result.add(LanguageToolRuleMatch.fromLanguageTool(match, annotatedTextFragment))
}

return result
Expand Down Expand Up @@ -206,11 +198,6 @@ class LanguageToolJavaInterface(
}
}

override fun disableRules(ruleIds: Set<String>) {
val languageTool: JLanguageTool = (this.languageTool ?: return)
languageTool.disableRules(ruleIds.toList())
}

override fun enableEasterEgg() {
val languageTool: JLanguageTool = (this.languageTool ?: return)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

package org.bsplines.ltexls.languagetool

import com.google.gson.JsonElement
import com.google.gson.JsonObject
import org.bsplines.ltexls.parsing.AnnotatedTextFragment
import org.bsplines.ltexls.server.LtexTextDocumentItem
import org.bsplines.ltexls.tools.Tools
Expand Down Expand Up @@ -34,8 +36,10 @@ data class LanguageToolRuleMatch(
companion object {
private val TWO_OR_MORE_SPACES_REGEX = Regex("[ \n]{2,}")

fun fromLanguageTool(match: RuleMatch, annotatedTextFragment: AnnotatedTextFragment):
LanguageToolRuleMatch {
fun fromLanguageTool(
match: RuleMatch,
annotatedTextFragment: AnnotatedTextFragment,
): LanguageToolRuleMatch {
return fromLanguageTool(
match.rule?.id,
match.sentence?.text,
Expand All @@ -48,6 +52,29 @@ data class LanguageToolRuleMatch(
)
}

fun fromLanguageTool(
jsonMatch: JsonObject,
annotatedTextFragment: AnnotatedTextFragment,
): LanguageToolRuleMatch {
val fromPos: Int = jsonMatch.get("offset").asInt
val suggestedReplacements = ArrayList<String>()

for (replacement: JsonElement in jsonMatch.get("replacements").asJsonArray) {
suggestedReplacements.add(replacement.asJsonObject.get("value").asString)
}

return fromLanguageTool(
jsonMatch.get("rule").asJsonObject.get("id").asString,
jsonMatch.get("sentence").asString,
fromPos,
fromPos + jsonMatch.get("length").asInt,
jsonMatch.get("message").asString,
suggestedReplacements,
RuleMatch.Type.Hint,
annotatedTextFragment,
)
}

@Suppress("LongParameterList")
fun fromLanguageTool(
ruleId: String?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ class CodeActionGenerator(
document, newWord, acceptSuggestionsMatches)))
}

if (addToDictionaryMatches.isNotEmpty()
&& this.settingsManager.settings.languageToolHttpServerUri.isEmpty()) {
if (addToDictionaryMatches.isNotEmpty()) {
result.add(Either.forRight(getAddWordToDictionaryCodeAction(document,
addToDictionaryMatches, annotatedTextFragments)))
}
Expand Down
8 changes: 0 additions & 8 deletions src/main/kotlin/org/bsplines/ltexls/settings/Settings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,6 @@ data class Settings(
return differences
}

if (dictionary != other.dictionary) {
differences.add(SettingsDifference("dictionary", this.dictionary, other.dictionary))
}

if (disabledRules != other.disabledRules) {
differences.add(SettingsDifference("disabledRules", this.disabledRules, other.disabledRules))
}

if (enabledRules != other.enabledRules) {
differences.add(SettingsDifference("enabledRules", this.enabledRules, other.enabledRules))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ class SettingsManager {
value.getDifferencesRelevantForLanguageTool(oldSettings)

if (settingsDifferencesRelevantForLanguageTool.isEmpty()) {
this.languageToolInterface = this.languageToolInterfaceMap[newLanguage]
val languageToolInterface: LanguageToolInterface? =
this.languageToolInterfaceMap[newLanguage]
this.languageToolInterface?.dictionary = value.dictionary
this.languageToolInterface?.disabledRules = value.disabledRules
this.languageToolInterface = languageToolInterface
} else {
if (Logging.logger.isLoggable(Level.FINE)) {
logDifferentSettings(newLanguage, settingsDifferencesRelevantForLanguageTool)
Expand Down Expand Up @@ -100,7 +104,6 @@ class SettingsManager {
}

languageToolInterface.enableRules(this.settings.enabledRules)
languageToolInterface.disableRules(this.settings.disabledRules)

this.languageToolInterface = languageToolInterface
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/kotlin/org/bsplines/ltexls/settings/SettingsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ class SettingsTest {

settings = settings.copy(_allDictionaries = settings.getModifiedDictionary(setOf("dictionary")))
assertEquals(setOf("dictionary"), settings.dictionary)
settings2 = compareSettings(settings, settings2, true)
settings2 = compareSettings(settings, settings2, false)

settings = settings.copy(
_allDisabledRules = settings.getModifiedDisabledRules(setOf("disabledRules"))
)
assertEquals(setOf("disabledRules"), settings.disabledRules)
settings2 = compareSettings(settings, settings2, true)
settings2 = compareSettings(settings, settings2, false)

settings = settings.copy(
_allEnabledRules = settings.getModifiedEnabledRules(setOf("enabledRules"))
Expand Down

0 comments on commit bcbad28

Please sign in to comment.