ankitects / anki

Anki's shared backend and web components, and the Qt frontend
https://apps.ankiweb.net
Other
18.32k stars 2.1k forks source link

Main Window Rework: How should I deprecate unused modules? #2474

Closed kleinerpirat closed 5 months ago

kleinerpirat commented 1 year ago

I have refactored a major part of Anki's front-end and merged a couple of formerly external windows into the main window. In the process, many files like deckbrowser.py, overview.py, studydeck.py, customstudy.py, the toolbar classes etc. are not needed anymore and I'd like to delete them. The webview is populated from a new file called mainpage.py.

What is the best practice to minimize add-on breakages due to failed imports? I saw the methods in anki._legacy, but these are for deprecating single functions and redirecting camelCase to snake_case.

It would be wonderful if someone gave me an example, say for aqt.deckbrowser.DeckBrowser. The new class is aqt.mainpage.MainPage. Not all methods of the former class are included in the new one. I'm ready to write redirection shims for depended-upon functions, but I don't know how to start.

(If you don't think it's worth the effort to minimize breakages for a 300 file diff - because some might be unpreventable -, I'd be happy too.)

Edit: Here's a little sneak peek:

image

kleinerpirat commented 1 year ago

@glutanimate friendly ping because I know you're an expert on add-on compatibility.

dae commented 1 year ago

The least disruptive approach is to have a period where both the old and new implementations are available, and allow the active one to be switched with an env var or flag + a restart. That will allow us to start with it off by default so that beta testers can opt in and easily revert if any showstopper problems are found, and will allow users who depend on old add-ons to continue to use the old path while we allow time for add-ons to get updated. Doing this will likely require a combination of files that check the flag to decide which routine to call, and some files may need two different versions for the two paths when they are radically different.

kleinerpirat commented 1 year ago

When I started this work I was hoping to avoid this, but now it seems like the easiest solution. I just hope we don't end up in a situation like the one we got with the stats screen.

kleinerpirat commented 1 year ago

Alright, I separated the two paths. main.py is radically different and needs to coexist with a new version using the NEXT suffix:

Other modules can do with the occasional check for the env var or are simply not used by mainNEXT. If it's okay, I will remove some legacy support from the NEXT codepath. My thinking is that a hard cut for the new main view might actually lead more add-on authors to update their code than if we continue patching things for them.

My plan for the next days/weeks is to separate the theme support changes from the design overhaul and publish those first. Wallpapers will be supported only for the NEXT version, because the old codepath causes ugly flashing due to webview reloads and is cumbersome to support anyway. Custom color palettes will be supported on both legacy and NEXT.

dae commented 1 year ago

main.py is a large file that contains a fair amount of code not directly related to the main window, so having two copies of it could present a considerable maintenance burden. Back when I originally introduced the v2 scheduler, I did so by copying the v1 scheduler's code into a different file and modifying it, and I lived to regret it - it ended up sticking around for a lot longer that originally anticipated, and bugfixes and updates frequently had to be applied twice - once to each of the files.

How do the two implementations differ? Are they mainly using different functions, or do they use the same function name that does different things? One approach you could use to keep the implementations fairly separate but avoid duplicating identical code, is to make the new implementation inherit from the old one, eg class MainWebViewNext(MainWebView).

(mainNEXT is not standard formatting - something like main_next.py would be better, or even main_window.py).

I just hope we don't end up in a situation like the one we got with the stats screen.

The stats screen doesn't concern me too much at the moment - the old code is fairly isolated and is not much of a burden. I think we could push the migration forward by providing a good example add-on and providing a clear timeline for when the old code will be dropped, but as it's not currently a burden, it hasn't bubbled to the top of the todo list.

If it's okay, I will remove some legacy support from the NEXT codepath. My thinking is that a hard cut for the new main view might actually lead more add-on authors to update their code than if we continue patching things for them.

That's fine - one of the advantages of parallel implementations is that the new implementation isn't as burdened by backwards compatibility concerns, as the old implementation covers that case during the migration period.

Wallpapers will be supported only for the NEXT version, because the old codepath causes ugly flashing due to webview reloads and is cumbersome to support anyway. Custom color palettes will be supported on both legacy and NEXT.

That's fine too - it's less work, and it gives people incentives to update.

kleinerpirat commented 1 year ago

Are they mainly using different functions, or do they use the same function name that does different things?

Mostly the latter, same function name but different way of achieving what the function name implies.

You're right, copying all of the code is an unnecessary burden for what I want to achieve. Subclassing AnkiQt and redefining the changed methods seems like the way to go. I had erroneously assumed new features would only be applied to the new codepath anyway, but of course things like bugfixes would need to be applied to both.


Sadly, the whole project took longer than expected due to ginormous feature creep and reality is catching up once again. I will take some time off to pass upcoming exams. In the evening I will give you the PR with the theme changes though, so you can see what has to get done in the backend.

nairyosangha commented 1 year ago

Just wondering as an end user, does this change make it impossible to have multiple windows with different views open at the same time? If so, that'd be pretty inconvenient for desktop users, what's the rationale for doing it like that?

tatsumoto-ren commented 1 year ago

Main Window Rework

Why is this needed? Is main windows broken in any way? If not, I suggest leaving it as is. If there is a bug, fix it with the shortest diff you can come up with.

merged a couple of formerly external windows into the main window.

Why would you do this?

What is the best practice to minimize add-on breakages due to failed imports?

As Dae said, implement your changes as an add-on.

screenshot-2023-04-18-20-03-57

If your add-on ends up being popular, you may ask to merge it into Anki itself, like it was done with the frozen fields.

I think forcing a major change without implementing it as an add-on first and without at least 6 months of testing is going to lead to horrible consequences. I've seen it before and it wasn't pleasant.

The least disruptive approach is to have a period where both the old and new implementations are available

In the past this approach led to the old implementation becoming more difficult to use. One example is the "Deck Options" dialog which is better than the newer one yet you have to shift+click to use it. Another example is the "Stats" window. Having to shift+click every single time is rather annoying. So I'd rather not repeat the same mistake. But, when choosing this path, the best solution would be to provide a checkbox under Preferences and give some sort of a promise that the old implementation won't suddenly disappear one day.

My thinking is that a hard cut for the new main view might actually lead more add-on authors to update their code than if we continue patching things for them.

Most add-on authors are not around anymore. They will never come back. Consequentially, it is important to keep all old APIs intact. Imagine if you printer, scanner, monitor or graphics card stopped working because someone decided so.


Anki has been quite stable in the past year or so, and I really appreciate that. But only If you don't count UI changes. Every time we had to fix the AJT add-ons recently, it was because Matthias did something. I don't want another tragedy to happen again. I'm sure this main window rework is pretty but at least keep it as an add-on for 6 months or so before merging into Anki.

kleinerpirat commented 1 year ago

Is main windows broken in any way?

Yes, it's ugly and looks like HTML without styling. Also we've been talking about merging the toolbar webviews with the main one for years and now I'm doing it.

merged a couple of formerly external windows into the main window.

Why would you do this?

Because Anki opens up a ridiculous amount of windows for actions that shouldn't require them.

@tatsumoto-ren I really don't feel like dealing with you at the moment ever. If you want a short answer for why Anki needs this: Damien wants to move away from Qt and for that Anki needs to be entirely translated to TS/Svelte. As for why I'm doing it: because I enjoy it.

That's all I'm going to say to you. I know you and your limited, egoistical world view and don't think anything fruitful will come from interacting with you any further. To anyone reading this and thinking I'm overreacting to seemingly mild criticism here: search up this guy on the forum or Anki's repo and you might understand where I'm coming from.


does this change make it impossible to have multiple windows with different views open at the same time?

@nairyosangha No. You will be able to open up major views like the stats and deck options screens as a separate window by holding Ctrl (or Shift, whatever users prefer).

nairyosangha commented 1 year ago

@nairyosangha No. You will be able to open up major views like the stats and deck options screens as a separate window by holding Ctrl (or Shift, whatever users prefer).

Okay nice, that sounds reasonable. I have the review, edit and card/note browser window (not sure if those are major views?) open at the same time very often, so I was wondering if I'd lose that ability. Thanks

tatsumoto-ren commented 1 year ago

@kleinerpirat you’re free to create and distribute your own fork of Anki if you don't like the current implementation. Alternatively, you can create an add-on and distribute that. If your add-on becomes popular, it may be considered for merging into Anki. But please stop rushing the project in a direction that makes other people suffer before the changes are tested for at least 6 months. Because of you I had to fix the AJT add-ons many times, and I'm sick of it. I hoped you would never submit another PR to Anki after the last time, but it seems you can't learn.

it's ugly and looks like HTML without styling.

If "ugly" is the reason then you need to stop right now. Everybody has different taste. It's perfectly fine to distribute different styles as add-ons. You can find examples of add-ons that change the looks of the program on AnkiWeb.

kleinerpirat commented 1 year ago

review, edit and card/note browser window (not sure if those are major views?)

@nairyosangha The editor and browser will continue to be separate windows on desktop by default. I might implement an option to open the "Edit Current" screen as a panel rather than a separate window, but it will not be forced upon anyone. The mobile clients AnkiMobile and AnkiDroid are increasingly using TS code from the desktop version, so those might benefit from such an addition.

What I am doing currently is reduce the amount of popup dialogs like "Study Deck", "Edit Description", "Custom Study", "Filtered Deck" and so on. As unaccessible as those are, I'd assume the average user is oblivious to those anyway and I'm hoping some of Anki's features will become more discoverable through my rework.

nairyosangha commented 1 year ago

review, edit and card/note browser window (not sure if those are major views?)

@nairyosangha The editor and browser will continue to be separate windows on desktop by default. I might implement an option to open the "Edit Current" screen as a panel rather than a separate window, but it will not be forced upon anyone. The mobile clients AnkiMobile and AnkiDroid are increasingly using TS code from the desktop version, so those might benefit from such an addition.

What I am doing currently is reduce the amount of popup dialogs like "Study Deck", "Edit Description", "Custom Study", "Filtered Deck" and so on. As unaccessible as those are, I'd assume the average user is oblivious to those anyway and I'm hoping some of Anki's features will become more discoverable through my rework.

Thanks for elaborating, that all sounds fine to me, seems like I got the wrong idea based on the screenshots :smile:

dae commented 1 year ago

@tatsumoto-ren Matthias has invested a large amount of time and energy into contributing code to Anki, including a fair amount of time adding features he doesn't personally use like 'reduce motion' and the minimalist mode to try to minimize the impact of the changes he made on people who preferred a simpler look. I can totally understand why your comments would frustrate him - you devalue his hard work, and come across as if you're entitled to bugfix-only updates. This is open source, and you are not owed anything. We do our best to minimize the impact of changes, but it's not reasonable to expect us to put a freeze on Anki's development because you don't want to have to sometimes update your add-ons.

Main Window Rework

Why is this needed?

There are multiple reasons:

As Dae said, implement your changes as an add-on.

That was the answer I gave you when you were advocating we waste resources updating an old code path that will eventually be retired. This is quite a different situation: Matthias is trying to move us forward here, and is working on a fairly central part of Anki that is not easily implemented in an add-on. And as I suggested above, the new code will be in new files and default to off to start with, which will allow us to minimize disruptions and have an extended testing period / phase out period.

glutanimate commented 1 year ago

@tatsumoto-ren as much as I understand the frustration of having to keep pace with Anki over time, I think it's undeniable that – in order for Anki to progress and evolve – add-on APIs have to be broken sometimes. Optimistically, each breakage also holds an opportunity to build up better APIs and build add-ons that are more resilient to change. Pessimistically, breakages can hold the risk of making Anki less modifiable by add-ons and discourage add-on development, which I believe would strongly devalue one of Anki's key value propositions.

In that light, it's completely fair to be apprehensive about these shifts as an add-on user and developer, but I think there's a right way and wrong way to voice your concerns, and a balance to be struck between Anki progress and add-on stewardship. From having followed these discussions for the second time now, I don't think that what you're doing is constructive. Your comments in this issue in particular feel very personal in addressing Matthias and – I have to fully agree with Damien – very disparaging of his work. Outright dismissal simply isn't a viable foundation for discussion. It's also slightly confusing to me because, if anything, the toolbar discussion should have shown that compromises can be reached.

Perhaps the greater context of why a rework of the main window is inevitable was not as apparent, but hopefully that should now be present with Damien's elaboration. So my personal hope would be that this will inspire a more grounded and less polemic discussion of these changes and redesign.


Responding to the actual ping:

Feature flags vs. hard break

@kleinerpirat, I would agree with @dae: Due to the extent of the changes, it seems like a feature flag and thus transition period would make the most sense. However, I think it's important not to forget that this will also incur an added maintenance cost on add-on authors, as they will likely have to maintain multiple implementations for an extended period of time. The redesign is fairly drastic, so even without add-on compatibility at play, I think we will see a significant chunk of the user base stick to the current design out of preference and habit. Though I guess that makes it all the more important to have a soft transition period, so +1 on a feature flag.

With that said, even with a feature flag present, I would hope that these changes go through an extended period of beta testing, both to allow for more feedback given their magnitude, and to allow add-on authors the ability to support both main window variants day one if they want to.

Regarding maintaining add-on compatibility in the new implementation

I think we should try to pursue this to our best efforts, but not further than it's sensible. The changes here appear major enough that I would assume that maintaining compatibility for a lot of python GUI hooks targeting the main window will either be very difficult or impossible to do. In particular because maintaining compatibility for a hook is only part of the story as add-ons also expect to be able to access Python code paths that this implementation likely completely transitions to TS.

Where we can cheaply maintain compatibility for existing Python hooks, let's do it; but in cases where extensive shims would be needed, and add-ons would break anyways due to non-hook changes, I would rather have us spent the effort towards building up new TS-based APIs.

Once you have the PR or branch ready, I think it would be nice to do a pass over existing hooks in aqt.gui_hooks, determine which ones are actually used frequently, also look at non-hook access to the main window views, and then map out the key add-on needs based on that (e.g. add-on dev stories like "I want to add a widget to the overview"). Using that map we could then build out new APIs that cater to these needs. I'd be happy to help on both of these fronts, of course.

Revisiting the developer experience for adding content to TS-based web views as part of that would also be nice. IMHO this still isn't up to par to the simplicity of Python hooks like webview_will_set_content or overview_will_render_content, with a lot of glue code required to inject scripts and content. As the main window is also one of the areas that's most frequently targeted by add-ons, we will have to consider how to provide sensible APIs that avoid add-on conflicts created by home-cooked DOM modifications. Things like providing TS type completions for these APIs would also be nice, of course.

All of this so far chiefly applies to add-ons that explicitly target the main window, but there is also a much larger group of add-ons out there that interact with the main window as a global entry point to various Anki objects like the collection. This I think is an area where we have to take care not to introduce any unnecessary breakages. As a silly example, if add-ons can no longer do

from aqt import mw

collection = mw.col

that would likely break hundreds of add-ons at once.

There are other access patterns like this that are fairly common in add-on development and, if not completely unavoidable, we should make sure not to break them, building out shims if necessary. But subclassing from AnkiQt should have you covered there mostly.


Regarding the redesign itself

Looking at the screenshots, I think it's clear that you've put a lot of thought into the redesign (on top of the impressive web view unification work). I really like some of the ideas present here, but also have some core concerns, in particular regarding the split-pane view and emphasis on decks, and also some questions, e.g. how this plays with the reviewer. However, I feel like this issue has already veered quite far from its original scope and don't want to contribute to steering it away even further (I also really don't want to take your mind off upcoming exams more than this discussion might have already done).

Once you're done with the theme support changes (and this exam period is hopefully past), how would you feel about an early feedback round on the redesign? Too many opinions at too early a stage typically aren't helpful of course, but the issue tracker here on GitHub seems like small enough of a round where this would be manageable. I could see this inspiring some helpful changes that might be easier to implement early on. Given the extent of the redesign, I think that separating this discussion out from the implementation discussion, and tackling the former first, would be prudent. What do you think?

kleinerpirat commented 1 year ago

@dae @glutanimate Thank you for chiming in. I felt bad about my response to Tatsumoto's concerns for the past two days and especially bad about you two having to come in as mediators. This could have been a perfectly civil discussion had I not reacted in such a toxic way.

You see @tatsumoto-ren, I have been working on this for several months now. I didn't do anything else but work on this ~12 h/d during the easter break and I'm pushing the limit of what I can allow myself without failing my exams. I'm not at technical uni anymore and my med friends don't have the slightest clue of what I'm doing. It hits hard when there's pushback from the only group that understands the effort that goes into development. Your reply came at a moment I wasn't stable enough to rethink my initial impulse. I'm very sorry about the bad blood that resulted from this.

I will keep my personal opinions out of this and treat you equal to other voices in this discussion. It is not fair towards the community if I play off valid concerns from your side just because I don't like you.


@glutanimate I'm all for a transition period and I'm not in a hurry to get the project to the masses. It can stay an opt-in experimental feature as long as necessary to polish it and reach feature-parity regarding add-on extension.

The codepaths are already separated and AnkiQtNext inherits from AnkiQt. That way only 450 lines are required for the new main_next.py instead of ~1700. Regarding existing hooks: I preserved the toolbar hook used by many add-ons and will try my best to either adapt to existing hooks or create alternatives for them.

I'd love to write up all major changes of the project, but I'm lacking the time to do it properly right now. You asked about the deck browser sidebar:

image

Please note these screenshots show a very early implementation and are subject to change. E.g. the study button will be moved to the middle, perhaps merged with the pie chart that indicates both the amount and type of due cards.

It is actually not a resizable panel, but part of the main grid layout. It has a fixed size (depending on screen real estate) and collapses when you click outside. It's got a search bar to filter decks and clicking the magnifying glass icon opens an AnkiWeb search for the input text. I plan to write an API for additional columns next to the basic three. Another feature I have in mind is multi-select for drag and drop and creating filtered decks quickly.

The main area to the right of the deck browser is where formerly external views are displayed: Overview (+ Edit Description), Custom Study (+ Filtered Deck), Deck Options, Stats. Like mentioned in an earlier comment, the deck options and stats can be opened as separate windows with a keyboard modifier.

The reviewer is translated to Svelte, spans the whole screen (position: fixed) inside an <iframe> and communicates with its host component via postMessage.

You might have also noticed the UI at the bottom left:

image

That's a profile selector for quick switching between profiles without leaving the main screen. The sun icon at the top-right of the screen is a toggle to temporarily switch the theme. In contrast to the preferences setting, this will not be remembered across sessions. The rework includes dozens of other changes like that intended to make the UX more streamlined and off the top of my head I cannot name them all here.

tatsumoto-ren commented 1 year ago

@dae @glutanimate I was a little shocked when I first found this issue. If I remember correctly, Damien said that changes should be small, and this change appears to be very big. As a result, in my first comment I come off rather confused.

screenshot-2023-04-20-15-54-41


Regarding add-on compatibility

Do you agree that an Anki add-on is something you write once and don't have to touch again? Personally, I feel this way. What would you say if the Linux kernel stopped supporting all drivers that were written before 2022? I would be frustrated. With Anki, this is pretty much the case. Sometimes an add-on that was written just a year ago doesn't work anymore.

Most add-on authors are not around anymore, and they will never come back. That's just the unfortunate reality that needs to be taken into account.

I think it's undeniable that – in order for Anki to progress and evolve – add-on APIs have to be broken sometimes.

Maybe if APIs have to be changed, the changes need to be introduced between major versions? For example, Anki 2.2 could introduce a new set of APIs, and then they would stay unchanged for the entire lifetime of 2.2.x, meaning no existing APIs are invalidated.

you devalue his hard work, and come across as if you're entitled to bugfix-only updates.

I'm sorry if it sounded this way. But to an extent I do believe that for the lifetime of Anki 2.1 older interfaces need to be available, while newer interfaces could be either disabled by default or be present in the 2.2 builds. This constraint does not apply to the backend though, it can move its own way.

it's not reasonable to expect us to put a freeze on Anki's development because you don't want to have to sometimes update your add-ons.

Have you thought of continuing the development in a different branch, and have it due for release in a year from now, for example?

breakages can hold the risk of making Anki less modifiable by add-ons and discourage add-on development

Yes, I agree with that.

Main Window Rework

For now it seems like the changes discussed in this issue are not worth adding into vanilla Anki. The add-on API is built for exactly that purpose. Let's release an add-on, and merge it into Anki if it becomes popular, like it was done with other add-ons.

Alternatively, these changes could be contributed to the https://ankiuser.net/study/ website. This way the desktop version remains stable and unaffected. Since the web version doesn’t use add-ons, any changes are acceptable, and there are no worries about compatibility.

That was the answer I gave you when you were advocating we waste resources updating an old code path that will eventually be retired.

I was willing to do all the work myself.

working on a fairly central part of Anki that is not easily implemented in an add-on.

If it's impossible to release it as an add-on, can it be done in the 2.2 branch?

Your comments in this issue in particular feel very personal in addressing Matthias and – I have to fully agree with Damien – very disparaging of his work.

I didn't intend them to feel personal. The last time I spent considerable effort fixing our add-ons. When I found this issue, the thought of having to fix them again caused me a lot of pain.

it seems like a feature flag and thus transition period would make the most sense.

Honestly the idea of having a transition period doesn't seem right to me because it implies eventually removing the old code. The old UI is perfectly fine. The alternatives are:

we will have to consider how to provide sensible APIs

A pretty robust approach is to provide hooks that allow the end user to add new elements without having to deal with any TS (hooks that are not depended on the underlying implementation). This is a good example.

I have been working on this for several months now. I didn't do anything else but work on this ~12 h/d during the easter break

I deeply appreciate your work.

Splitting the GUI and the backend

Splitting the GUI and the backend seems like a good idea. If done, it will be possible to start developing a new GUI for Anki from scratch, independently from ankitects, using a completely different set of technologies, thus finally throwing away all TS and svelte.

To give you an example, the Coolreader book reader is based on the Crengine library. But there are many other book readers that are based on the same library, such as Koreader.

I have a plan to create a drop-in replacement app for Anki, such that it would be able to sync with AnkiWeb and read your current Anki collection. It would be implemented with no JS,TS,Svelte, and its interface would be more similar to Anki 2.0.

kleinerpirat commented 1 year ago

What would you say if the Linux kernel stopped supporting all drivers that were written before 2022? I would be frustrated. With Anki, this is pretty much the case.

I'd assume there are more than five people working on the Linux kernel?

I deeply appreciate your work.

Right.

I have a plan [...] no JS,TS,Svelte, and its interface would be more similar to Anki 2.0

The replacement app sounds like a great idea though. You'd be doing the community a big favor.

tatsumoto-ren commented 1 year ago

I'd assume there are more than five people working on the Linux kernel?

If what you're trying to say is that there aren't enough people working on Anki, that's actually a good reason to be more conservative when introducing further changes in order to maintain a stable API.

The replacement app sounds like a great idea though.

It seems many projects adopt this approach. Another example is Trackma — it has a Qt interface, a GTK interface, and two command-line interfaces.

dae commented 1 year ago

Do you agree that an Anki add-on is something you write once and don't have to touch again?

No, I don't think that's a reasonable expectation. There is a spectrum of possible add-on support:

  1. No add-on support. You need to fork the project to implement your own features, and you are responsible for integrating any changes and bugfixes that are made upstream, and distributing your own builds if you wish to share your work with others.
  2. A set of well-defined hooks and static interfaces that add-ons can use. These require careful planning to get right and are time-consuming to add, so the possible things you can do is likely to be limited. The upside is that they're less likely to break as the app evolves, as the contract is well defined.
  3. Allowing add-ons to dynamically replace parts of the code, such as monkey patching and replacing HTML. This allows the most flexibility to add-on authors as it allows them to do things the original authors did not anticipate, but is the most fragile, as potentially any code change could break someone's hack.

We try not to break things where possible (at considerable time and effort), but when add-ons are potentially relying on any of the private methods, variables and GUI/page layouts we're using, it's almost impossible to evolve the codebase without breakages happening. You can reduce the likelihood of breakages by relying on hooks, and adding new ones when required, as e.g. @glutanimate has done in the past, though there will still be cases where even hooks/interfaces need to change.

Maybe if APIs have to be changed, the changes need to be introduced between major versions?

The reason why Anki doesn't use semver has been discussed on the forums. The reality is that we already have major and minor releases, and the larger updates go through longer periods of development/alphas/betas - 2.1.55 took about 6 months. The longer a period of development though, the harder maintenance becomes, as it becomes harder to port bugfixes between development and stable branches when the code diverges.

Likewise with the "you should keep the old stuff around forever" comments. Legacy code has a maintenance cost, and it's not practical for us to support it forever, especially as users are free to keep using older versions if they wish.

I have a plan to create a drop-in replacement app for Anki, such that it would be able to sync with AnkiWeb and read your current Anki collection. It would be implemented with no JS,TS,Svelte, and its interface would be more similar to Anki 2.0.

I suspect you'll find that's more work than occasionally updating your add-ons, but if means an end to the disparaging comments and demands that we avoid changing anything, that will be a relief. If you do go down that route, please change the name and provide your own support site, so it doesn't cause users confusion or increase our support burden.

tatsumoto-ren commented 1 year ago

You can reduce the likelihood of breakages by relying on hooks

This is an interesting suggestion. I estimate that transferring our entire codebase to using hooks will require Anki to accept a dozen of PRs from me, or a few big PRs. Knowing that you reject PRs quite often, I've been abstaining from doing it until now. But if you're willing to do it, it will be definitely easier and will lower the possibility of breakage.

Currently hooks are very limited in what they can offer, so often in the past we couldn't use them in our add-ons.

There are existing hooks that don't bring any stability benefits, they're no better than using wrap(). For example, gui_hooks.webview_will_set_content. Though the end-user can utilize it, every time the default HTML changes in a new anki release, it is necessary to redo the add-on code again. This suggests that more high-level hooks are necessary, such that you can modify elements presented as instances of some dataclass, or as tuples. reviewer_will_init_answer_buttons is a good hook when looking from this perspective actually.

Often functions in Anki's code output just raw HTML. For example Reviewer._bottomHTML. Simply adding hooks to them won't help because the HTML that they output can change in a new release. Their internals need to be refactored towards using more abstract objects first.

when add-ons are potentially relying on any of the private methods, variables and GUI/page layouts

I expect there are many such add-ons. The GUI has been very unstable recently which doesn't help. There are a few places where breakages don't happen, such as the editor toolbar, because you can call editor.addButton(). However, all other parts of the editor aren't covered.

disparaging comments

Regarding my "disparaging comments", I think they've been pretty on point while being modest and restrained at the same time. Most importantly, I was able to point out the parts where improvements are needed in order to lower maintenance cost for add-on developers, and I was able to solve some of the issues thanks to your help.

avoid changing anything

Till now there have been no alternatives to this. In principle I don't mind changes. Now that you've suggested adding more hooks, it might work if enough hooks are added. I'll look into my codebase and try to come up with hooks that I can request in a PR.

Matthias said that he is not a fan of add-ons and instead prefers to shove all his ideas directly into Anki. This approach has caused inconvenience before, to put it mildly. How about he tries to implement his changes as an add-on? If he's unable to do it, let him first make the hooks necessary. If the changes can be recreated in an add-on using hooks, then it makes sense to merge them into Anki. It should also make undoing undesirable changes easier using the addon API.


After researching the existing code, I found that places where we need hooks the most are functions that return html as strings. Functions like Reviewer._answerButtons, Reviewer._showAnswerButton, Reviewer._bottomHTML, Reviewer._remaining. There is likely little benefit in having hooks if the html they output can change across releases.

kleinerpirat commented 1 year ago

Functions like Reviewer._answerButtons, Reviewer._showAnswerButton , Reviewer._bottomHTML , Reviewer._remaining

All of these are already replaced with a more abstract hook in the mw rework.