Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for Duplicates In Mining Mode #109

Merged
merged 11 commits into from
Dec 12, 2024
Merged

Check for Duplicates In Mining Mode #109

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e410754
Check for Duplicates In Mining Mode
bpwhelan Nov 25, 2024
850b34e
Remove alert for Dupe detection
bpwhelan Nov 25, 2024
0d51e21
Do Duplicate Check After the fact
bpwhelan Nov 27, 2024
5b9e33b
Remove unused import
bpwhelan Nov 27, 2024
a687c8d
PR Review Changes
bpwhelan Dec 5, 2024
4d42f6f
remove unused import
bpwhelan Dec 5, 2024
dbcc60a
PR Review Changes #3
bpwhelan Dec 11, 2024
c3483ce
remove debug
bpwhelan Dec 11, 2024
25abc48
Merge remote-tracking branch 'upstream/master'
bpwhelan Dec 12, 2024
a5aee43
PR Review Changes
bpwhelan Dec 12, 2024
ddcb427
Suggested primarySpellingandFirstReading change
bpwhelan Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions JL.Core/Config/CoreConfigManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public sealed class CoreConfigManager
public bool AnkiIntegration { get; set; } // = false;
public bool ForceSyncAnki { get; private set; } // = false;
public bool AllowDuplicateCards { get; private set; } // = false;
public bool CheckForDuplicateCards { get; private set; } // = false;
public double LookupRate { get; private set; } // = 0;
public bool CaptureTextFromClipboard { get; set; } = true;
public bool CaptureTextFromWebSocket { get; set; } // = false;
Expand Down Expand Up @@ -95,6 +96,7 @@ public void ApplyPreferences(SqliteConnection connection)
AnkiIntegration = ConfigDBManager.GetValueFromConfig(connection, AnkiIntegration, nameof(AnkiIntegration), bool.TryParse);
ForceSyncAnki = ConfigDBManager.GetValueFromConfig(connection, ForceSyncAnki, nameof(ForceSyncAnki), bool.TryParse);
AllowDuplicateCards = ConfigDBManager.GetValueFromConfig(connection, AllowDuplicateCards, nameof(AllowDuplicateCards), bool.TryParse);
CheckForDuplicateCards = ConfigDBManager.GetValueFromConfig(connection, CheckForDuplicateCards, nameof(CheckForDuplicateCards), bool.TryParse);
LookupRate = ConfigDBManager.GetNumberWithDecimalPointFromConfig(connection, LookupRate, nameof(LookupRate), double.TryParse);
TextBoxTrimWhiteSpaceCharacters = ConfigDBManager.GetValueFromConfig(connection, TextBoxTrimWhiteSpaceCharacters, nameof(TextBoxTrimWhiteSpaceCharacters), bool.TryParse);
TextBoxRemoveNewlines = ConfigDBManager.GetValueFromConfig(connection, TextBoxRemoveNewlines, nameof(TextBoxRemoveNewlines), bool.TryParse);
Expand Down
71 changes: 71 additions & 0 deletions JL.Core/Mining/MiningUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,77 @@ public static async Task MineToFile(LookupResult lookupResult, string currentTex
Utils.Logger.Information("Mined {PrimarySpelling}", lookupResult.PrimarySpelling);
}

public static async Task<bool> CheckDuplicate(LookupResult lookupResult)
{
CoreConfigManager coreConfigManager = CoreConfigManager.Instance;
if (!coreConfigManager.AnkiIntegration || !coreConfigManager.CheckForDuplicateCards)
{
return false;
}

Dictionary<MineType, AnkiConfig>? ankiConfigDict = await AnkiConfig.ReadAnkiConfig().ConfigureAwait(false);
if (ankiConfigDict is null)
{
return false;
}

AnkiConfig? ankiConfig;
if (DictUtils.s_wordDictTypes.Contains(lookupResult.Dict.Type))
{
ankiConfig = ankiConfigDict.GetValueOrDefault(MineType.Word);
}
else if (DictUtils.s_kanjiDictTypes.Contains(lookupResult.Dict.Type))
{
ankiConfig = ankiConfigDict.GetValueOrDefault(MineType.Kanji);
}
else if (DictUtils.s_nameDictTypes.Contains(lookupResult.Dict.Type))
{
ankiConfig = ankiConfigDict.GetValueOrDefault(MineType.Name);
}
else
{
ankiConfig = ankiConfigDict.GetValueOrDefault(MineType.Other);
}

if (ankiConfig is null)
{
return false;
}

Dictionary<string, JLField> userFields = ankiConfig.Fields;
Dictionary<JLField, string> miningParams = new()
Copy link
Owner

@rampaa rampaa Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should use GetMiningParameters here. The implementation provided here just partially creates mining parameters but for all we know the user might have SourceText as their note type's first field (which is all Anki cares for when checking if a card is a duplicate AFAIK) and its value is missing here.

Or better yet, we can check what's the first Field in the config and just create a note with that field to check whether it's a duplicate note.

{
[JLField.LocalTime] = DateTime.Now.ToString("s", CultureInfo.InvariantCulture),
[JLField.DictionaryName] = lookupResult.Dict.Name,
[JLField.MatchedText] = lookupResult.MatchedText,
[JLField.DeconjugatedMatchedText] = lookupResult.DeconjugatedMatchedText,
[JLField.PrimarySpelling] = lookupResult.PrimarySpelling,
[JLField.PrimarySpellingWithOrthographyInfo] = lookupResult.PrimarySpellingOrthographyInfoList is not null
? $"{lookupResult.PrimarySpelling} ({string.Join(", ", lookupResult.PrimarySpellingOrthographyInfoList)})"
: lookupResult.PrimarySpelling
};

Dictionary<string, string> fields = ConvertFields(userFields, miningParams);

// Audio/Picture/Video shouldn't be set here
// Otherwise AnkiConnect will place them under the "collection.media" folder even when it's a duplicate note
Note note = new(ankiConfig.DeckName, ankiConfig.ModelName, fields, ankiConfig.Tags, null, null, null, null);
if (!coreConfigManager.AllowDuplicateCards)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this check here? As is, this method will always return false if the user enabled the Allow duplicate cards option, is this really the intended behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already removed in local.

{
bool? canAddNote = await AnkiUtils.CanAddNote(note).ConfigureAwait(false);
if (canAddNote is null)
{
return false;
}

if (!canAddNote.Value)
{
return true;
}
}
return false;
}

public static async Task Mine(LookupResult lookupResult, string currentText, string? formattedDefinitions, string? selectedDefinitions, int currentCharPosition)
{
CoreConfigManager coreConfigManager = CoreConfigManager.Instance;
Expand Down
4 changes: 4 additions & 0 deletions JL.Windows/ConfigManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,7 @@ public void LoadPreferenceWindow(PreferencesWindow preferenceWindow)
preferenceWindow.WebSocketUriTextBox.Text = coreConfigManager.WebSocketUri.OriginalString;
preferenceWindow.ForceSyncAnkiCheckBox.IsChecked = coreConfigManager.ForceSyncAnki;
preferenceWindow.AllowDuplicateCardsCheckBox.IsChecked = coreConfigManager.AllowDuplicateCards;
preferenceWindow.CheckForDuplicateCardsCheckBox.IsChecked = coreConfigManager.CheckForDuplicateCards;
preferenceWindow.LookupRateNumericUpDown.Value = coreConfigManager.LookupRate;
preferenceWindow.KanjiModeCheckBox.IsChecked = coreConfigManager.KanjiMode;
preferenceWindow.AutoAdjustFontSizesOnResolutionChange.IsChecked = AutoAdjustFontSizesOnResolutionChange;
Expand Down Expand Up @@ -1213,6 +1214,9 @@ public async Task SavePreferences(PreferencesWindow preferenceWindow)
ConfigDBManager.UpdateSetting(connection, nameof(CoreConfigManager.AllowDuplicateCards),
preferenceWindow.AllowDuplicateCardsCheckBox.IsChecked.ToString()!);

ConfigDBManager.UpdateSetting(connection, nameof(CoreConfigManager.CheckForDuplicateCards),
preferenceWindow.CheckForDuplicateCardsCheckBox.IsChecked.ToString()!);

ConfigDBManager.UpdateSetting(connection, nameof(CoreConfigManager.LookupRate),
preferenceWindow.LookupRateNumericUpDown.Value.ToString(CultureInfo.InvariantCulture));

Expand Down
25 changes: 25 additions & 0 deletions JL.Windows/GUI/PopupWindow.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,31 @@ private StackPanel PrepareResultStackPanel(LookupResult result, int index, int r
audioButton.Click += AudioButton_Click;

_ = top.Children.Add(audioButton);

if (!configManager.MineToFileInsteadOfAnki)
rampaa marked this conversation as resolved.
Show resolved Hide resolved
bpwhelan marked this conversation as resolved.
Show resolved Hide resolved
{
if (MiningUtils.CheckDuplicate(result).Result)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing the Result property of a Task is a blocking operation, thus it should be avoided. Instead we should do something like

bool duplicateCard = await MiningUtils.CheckDuplicate(result).ConfigureAwait(true);
if (duplicateCard)

Copy link
Contributor Author

@bpwhelan bpwhelan Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the biggest thing that I'm having trouble with since this method is synchronous.

Toying with something to do the check for duplicates after the window is loaded and toggling the visibility so there is 0 impact on loading the window.

{
Utils.Frontend.Alert(AlertLevel.Error, $"{result.PrimarySpelling} is a duplicate card");
Button duplicate = new()
{
Name = nameof(duplicate),
Content = "⚠️",
Foreground = configManager.DefinitionsColor,
VerticalAlignment = VerticalAlignment.Top,
Margin = new Thickness(3, 0, 0, 0),
HorizontalAlignment = HorizontalAlignment.Left,
Background = Brushes.Transparent,
Cursor = Cursors.Arrow,
BorderThickness = new Thickness(0),
Padding = new Thickness(0),
FontSize = 14,
ToolTip = $"{result.PrimarySpelling} is already in anki deck."
};

_ = top.Children.Add(duplicate);
}
}
}

if (result.AlternativeSpellings is not null && configManager.AlternativeSpellingsFontSize > 0)
Expand Down
7 changes: 7 additions & 0 deletions JL.Windows/GUI/PreferencesWindow.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,13 @@
<CheckBox x:Name="AllowDuplicateCardsCheckBox" HorizontalAlignment="Right" />
</DockPanel>

<DockPanel Margin="0,10,0,0">
<TextBlock HorizontalAlignment="Left" Text="Check for duplicate cards"
Style="{StaticResource TextBlockDefault}" VerticalAlignment="Center" TextWrapping="Wrap"
ToolTip="Checks Anki for Duplicate Expressions when entering Mining Mode" Cursor="Help"/>
<CheckBox x:Name="CheckForDuplicateCardsCheckBox" HorizontalAlignment="Right" />
</DockPanel>

<TabControl Margin="0,10,0,0">
<TabControl.Resources>
<Style TargetType="{x:Type TabPanel}">
Expand Down