On modelchange used

[Arthur-Milchior]

in editor, the method onModelChange compute the new note... and that's all. It was forgotten to save it and put it as the new note of the editor.

My first commit solve this problem.

The other two commits simply clean up the code. In this case, I believe that a single try is better than two. And a if is even better than a try, since the only exception could be really easily detected in advance

[Arthur-Milchior]

I do recognize that it was my mistake, to see in 0b633db that setNote was called twice instead of once, and not to realize that the note itself should still be saved

[dae]

Did you test this Arthur?

[Arthur-Milchior]

I did test and it works. I created a note type "TEST" with "back", "middle" and "front", and switched from and to "basic". Back went to back, front to front. Middle's content is ignored or empty.

If furthermore I add to "Basic" a field "milieu" in second position, then "milieu" gets "middle" content and reciprocally.

The main problem I didn't realize while testing last time is that Anki had this feature: field content follow field names if possible, and field position otherwise. Since I never paid attention to this feature, I didn't even know I broke it. Not until I paid more attention to this part of the code due to a request I had for an add-on.

I recognize that doing those tests right now showed a mistake that I just corrected by a force push

[dae]

Before you force-pushed, you were comparing a number against a list. I've also just had to revert one of your other PRs from yesterday that is also incorrect. And I think there were about 3 separate issues i had to fix with your preview refactor. Please test your changes thoroughly before submitting them Arthur - we all make mistakes, but you are letting too many issues slip through.

[Arthur-Milchior]

To be clear: I did answer to this remark on #583
Once again, I'm extremely sorry.
I would just like to note that, whether or not you decide to accept this PR and maybe the corrected version of #583 (i.e. the branch https://github.com/Arthur-Milchior/anki/tree/remove_a_try ), the bug I described above is still present in anki.

As I told some time ago, I'm not going to make any more PR in the near future to anki anyway; given my new employment.

[Arthur-Milchior]

I don't understand the macos-latest check, but the failure seems unrelated to this PR

[evandrocoan]

@Arthur-Milchior, the Mac OS build is failing because of this bug pylint-dev/pylint#3542 (Latest update 2.5.0 broke --extension-pkg-whitelist) which was fixed on this master with ankitects/pylint#1 (Fix astroid update breaking build).

Somehow the Mac OS is using the old version (which was not fixed, maybe due to some cache?). Rebasing the master branch changes with this branch should fix the problem. To do this, I usually create a new branch from the master, cherry-pick all commits from my old branch, change the remotes and force push the new branch over the old branch on GitHub.

[Arthur-Milchior]

Is there any problem in simply rebasing over master ?

Thanks for the information, I didn't get this error

[evandrocoan]

I think that just running checkout on this branch and running git rebase master should work without problems, I just forget to try it out. But I know that running git merge master can sometimes cause the GitHub diff Interface as (https://github.com/ankitects/anki/pull/584/files) to double show the changes from this branch + the changes on the master branch (3828216#commitcomment-38246298).

[Arthur-Milchior]

I followed those last advice and just rebased. Seems more beautiful

[evandrocoan]

Now it is just failing because the Mac OS machines are nuts: (if you force push again it will probably pass as that failure is just random)

[Arthur-Milchior]

Is there no way to just tell a task to restart ? I can force push, but it seems a waste to force all other test to run again.

I can not really ask to be trusted in my PR. I can may be ask to ensure that the change I made here are only on backend and there is really no reason for a difference between mac and other OS. Especially given this error message

[evandrocoan]

At the moment, there is no way to tell GitHub Actions to rerun only a single machine. You can only return everything either by force pushing or closing and reopening the pull request. But reopening the pull request would cause this other bug: actions/runner#391 (Closing and opening a pull request doubles the number of jobs).

There is no need to rerun the tests because that failure happened after all tests have already passed (as the error happened after the tests had run, at the end of the workflow). I just mentioned it in case you like to see everything green).

[Arthur-Milchior]

I personally don't care about seing everything green, as long as there is a reasonable assumption that the red is unrelated to the PR.
I do not know what DAE's opinion about green and red is. I've been told to do a "make check" as a hook before pushing, it is done, and the push does not occur if the check failed on my computer. But appart from that, I don't know how strict he is about "all green"

[evandrocoan]

The only downside about not having all green is that DAE (or anyone else popping around here) will have to click on the red machine and see why it is failing (i.e., waisting time for nothing, as there is no actual failure). While if it is all green, he does not have to do that (i.e., no waste of time for no one other than the one creating the pull request which had to force-push).

[dae]

When single tests fail I check why, so don't worry about them preventing a merge. Sorry for the delay, I am really busy lately.

[Arthur-Milchior]

Really no problem. Anyway, I can't use the PR I create in my add-ons until the next version of anki is out. So the time you take to accept/reject them does not change my workflow.