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

Jump to bottom

- Added ContentWdw,MoveElemetToConcept #242

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

- Added ContentWdw,MoveElemetToConcept #242

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 19, 2020 •
edited by alexis-

No description provided.

@alexis- alexis- self-requested a review November 5, 2020 12:38
alexis-
alexis- requested changes Nov 5, 2020
View reviewed changes
Copy link
Member

@alexis- alexis- left a comment •
edited

Choose a reason for hiding this comment

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

  1. Can you fix the few issues I've outlined ?

  2. More globally, there are problems with the coding style (e.g. SMNatives.TContents, ContentsWdw, ...).

The easiest way to address this would be for you to install Resharper. It will load the project's coding style and automatically apply them. You can also run Resharper's "Refactor" function on existing code to reformat it (e.g. TNatives.TContents).

  1. For the rest, good work !

SuperMemoAssistant.sln Outdated
@@ -141,7 +141,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{6A546934-EB9
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.VersionBuilder", "src\Core\SuperMemoAssistant.VersionBuilder\SuperMemoAssistant.VersionBuilder.csproj", "{712820C7-6A72-4829-8D39-EDE2FDB32666}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.OmniMemo", "src\Plugins\SuperMemoAssistant.Plugins.OmniMemo\src\SuperMemoAssistant.Plugins.OmniMemo\SuperMemoAssistant.Plugins.OmniMemo.csproj", "{F737AECE-9DAB-40EB-A2EF-DE97F43BA87C}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.TestSetConceptNo", "..\SuperMemoAssistant.Plugins.TestSetConceptNo\src\SuperMemoAssistant.Plugins.TestSetConceptNo\SuperMemoAssistant.Plugins.TestSetConceptNo.csproj", "{BAC96BE4-668C-477C-B02B-C496147EEC3E}"
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove that Plugin ?

src/Core/SuperMemoAssistant.Core/NativeDataCfg.json Outdated
@@ -493,7 +498,7 @@
// TElWind::ForceRepetitionExt
"ElWdw_ForceRepetitionExt": "55 8B EC 81 C4 ? ? ? ? 88 4D F7 89 55 F8 89 45 FC 8D 4D 81",
// ???? TElWind::MIForgetClick or TElWind::MIJumpIntervalClick => Last call
"ElWdw_RestoreLearningMode": "55 8B EC 51 89 45 FC 8B 45 FC 80 B8 ? ? ? ? ? 75 1E"
"ElWdw_RestoreLearningMode": "55 8B EC 51 89 45 FC 8B 45 FC 80 B8 ? ? ? ? ? 75 1E",
Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be a comma at the end of the line

SuperMemoAssistant.sln Outdated
@@ -141,7 +141,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{6A546934-EB9
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.VersionBuilder", "src\Core\SuperMemoAssistant.VersionBuilder\SuperMemoAssistant.VersionBuilder.csproj", "{712820C7-6A72-4829-8D39-EDE2FDB32666}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.OmniMemo", "src\Plugins\SuperMemoAssistant.Plugins.OmniMemo\src\SuperMemoAssistant.Plugins.OmniMemo\SuperMemoAssistant.Plugins.OmniMemo.csproj", "{F737AECE-9DAB-40EB-A2EF-DE97F43BA87C}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.TestSetConceptNo", "..\SuperMemoAssistant.Plugins.TestSetConceptNo\src\SuperMemoAssistant.Plugins.TestSetConceptNo\SuperMemoAssistant.Plugins.TestSetConceptNo.csproj", "{BAC96BE4-668C-477C-B02B-C496147EEC3E}"
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to pull the latest version, the OmniMemo plugin is being erased

src/Core/SuperMemoAssistant.Core/SuperMemo/Common/Elements/ElementRegistryBase.cs Outdated
@@ -601,7 +601,7 @@ private static bool AddElement(ElementBuilder builder, out int elemId)
// Has a concept been specified for the new element ?

if (builder.Concept != null)
if (Core.SM.UI.ElementWdw.SetCurrentConcept(builder.Concept.Id) == false)
if (Core.SM.UI.ContentWdw.MoveElementToConcept(this[elemId].Id,builder.Concept.Id) == false)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a space after the comma ?

if (Core.SM.UI.ContentWdw.MoveElementToConcept(this[elemId].Id, builder.Concept.Id)

src/Core/SuperMemoAssistant.Core/SuperMemo/Natives/SMNatives.cs Outdated
@@ -63,6 +64,7 @@ public SMNatives(NativeData nativeData)
public TRegistry Registry { get; }
public TFileSpace FileSpace { get; }
public TQueue Queue { get; }
public TContents Contents { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Missing a space for alignment (coding style)

src/Core/SuperMemoAssistant.Core/SuperMemo/SuperMemo17/UI/ContentWdw.cs Outdated

public ContentWdw()
{
Core.SMA.OnSMStartedEvent += OnSMStartedEventAsync;
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace with OnSMStarting ?

src/Core/SuperMemoAssistant.Core/SuperMemo/SuperMemo17/UI/ContentWdw.cs Outdated


#region Methods
private async Task OnSMStartedEventAsync(object sender,
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace with OnSMStarting ?

src/Core/SuperMemoAssistant.Hooks.Common/SuperMemoAssistant.Hooks.Common.csproj Outdated
</ItemGroup>
<ItemGroup>
<PackageReference Update="Nerdbank.GitVersioning" Version="3.2.31" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Why was that added ?

Copy link
Author

Choose a reason for hiding this comment

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

I have no clue how but will remove it.

@alexis- alexis- added enhancement New feature or request sma-api SMA interface API labels Nov 5, 2020
@alexis- alexis- added this to the SMA 2.1.0 "Open-Beta" milestone Nov 5, 2020
@alexis- alexis- assigned ghost Nov 5, 2020
@ghost ghost requested a review from alexis- November 9, 2020 16:34
alexis-
alexis- approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

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

All good except the versions.json. When you commit make sure to compare all the files one by one.

I've updated them and I'm accepting the PR thanks !

@alexis- alexis- self-requested a review November 10, 2020 17:41
alexis-
alexis- requested changes Nov 10, 2020
View reviewed changes
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

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

I actually missed something during my review. The .sln seems a little messed up. At a very quick glance I identified:

  • You've added a x64 platform
  • Some configurations are missing (NuPack)

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement New feature or request sma-api SMA interface API
Milestone
SMA 2.1.0 "Open-Beta"
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alexis- @icepersi