jzohrab / lute

DEPRECATED: LUTE (Learning Using Texts) is a self-hosted web app for learning language through reading, based on Learning with Texts (LWT)
The Unlicense
118 stars 10 forks source link

Using real URL for dictionary URL #21

Closed HugoFara closed 1 year ago

HugoFara commented 1 year ago

Currently, the dictionary URL contain arbitrary placeholders, it has a few consequences:

I formatted everything with a proper URL in my fork of LWT, as it works better than the previous system I would like to apply a similar system here. Let me detail it.

General approach

As far as I have though, the best system is to replace arbitrary code by instructions preceded by a nice prefix like "lute_". For instance:

A bit more about Pop-Up marker

I don't think this part should go into the dictionary URL, as it makes it longer whatsoever and can be confusing for users. A field "display in pop-up", with a database counterpart can be better, but it is slightly more difficult to implement.

Backward compatibility

Backward compatibility with the "placeholder" system was not to hard to achieve, it shouldn't be harder to achieve here. I would also like to add a compatibility with my (prefixed by lwt_) if you don't have any objection against it.

If it makes sense to you, I can work on it and make a nice PR.

jzohrab commented 1 year ago

I'm on the fence with this idea, and feel that it's not immediately necessary -- the current code works -- so we should hold off until it's needed. BUT if there is a real immediate shortcoming with the current implementation, and not just nice-to-haves, let me know!

I totally agree that the leading asterisk isn't a great design -- I have an old TODO in the code to remove this thing, but it has never been a priority :-)

It's not difficult to manipulate the URLs, they're just strings and tokens. Again, the current code works and is not the cause of any real dev thrash. :-) If we replace tokens by other tokens, it's still the same kind of string-parsing effort.

With regard to it being a rigid system -- I haven't had the need yet to make it more flexible, and I don't want to implement things that only might be needed in the future. :-) Designing for the future is almost always a bad idea.

Some alternatives to this idea:

add "display in popup" checkbox

This would just require a db migration and change to the language form and processing. Wouldn't be that hard to do. :-) Is probably the right thing to do, as a first step.

a longer-term alternative -- more work though :-P - and probably should be put into its own GitHub issue

I think that rather than put more magic placeholders into the URL string, it would be better to revamp the way that dictionary entries are stored. The revamp will be more work, but will potentially open up other possibilities. e.g., rather than store a string with a bunch of behaviour-specifying flags, there could be a brand new user form like this:

These would be stored in a new dictionaries table, and would be linked to the languages. First draft UI implementation could be a dedicated UI screen to define dictionaries, that would be easiest (It's possible to create child subforms, but I haven't done that yet in Symfony :-) ).

One dict would have to be marked as primary. A language could define one or multiple dicts.

HugoFara commented 1 year ago

Basically I had to change the dictionary system when I wanted a closer integration with dictionaries such as LibreTranslate, now LWT supports auto-translation with this dictionary, and it can be expanded to other API quite easily.

For the popup, I did it for LWT (see picture), it was not hard and makes the life of users much easier. image It's up to you if you want me to do a similar system, or if you prefer to way for a dictionary cycle!

About the longer-term alternative, well, let see that in the future :smile:

jzohrab commented 1 year ago

Do you just use LibreTranslate for the auto-translate, or can users pick it as a dictionary as well?

If the former, then it's outside of the dictionary domain.

If the latter, then I can perhaps see why a change would be necessary -- but on first thought I'd likely go a different route with the design anyway, since the behaviour is completely different than a normal http dictionary web page. Different behaviour = different code class.

HugoFara commented 1 year ago

Users can use LibreTranslate as a dictionary as well, but it's a translator, so that's not recommended. LWT-fork currently has a unified syntax between dictionaries/translator, which is simpler for users, and strongly reduces code paths on dev side. Currently it serves as a one way-fit-all solution.

For me one of the biggest strength of Lingq is it's integration of dictionaries seamlessly, as LUTE is much versatile by nature a good handling of dictionaries saves time and tears. As you suggest a more profound change, do you have any precise system you want to implement?

jzohrab commented 1 year ago

Yeah the LingQ pre-defined dictionaries are handy, for sure. Most of their dicts are in pop-up windows, from what I've seen, so they're still just using URLs and token substitution of some sort.

I don't think removing special tokens from the URL is worth it, at the moment, so I don't want to do more work on dictionaries. We shouldn't do work just because it's possible. If there's a concrete need later, can revisit. The precise system is detailed above, at least my first idea of it :-) I haven't thought in depth on it because I don't have a concrete need for it.

LibreTranslate support is a non-trivial piece of work that should go into its own ticket.

HugoFara commented 1 year ago

We shouldn't do work just because it's possible. If there's a concrete need later, can revisit. I expected this kind of answer :smile: It was something that became necessary for LWT-fork, but it's as you wish for LUTE!

LibreTranslate: I agree with that, it was an example of use case, not something I wanted to tackle here.

So I think we can close this issue, maybe to restore it later on.