Skip to content
This repository has been archived by the owner on Nov 12, 2023. It is now read-only.

Using real URL for dictionary URL #21

Closed
HugoFara opened this issue Apr 13, 2023 · 6 comments
Closed

Using real URL for dictionary URL #21

HugoFara opened this issue Apr 13, 2023 · 6 comments

Comments

@HugoFara
Copy link
Contributor

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

  • The dictionary input filed cannot be set of type "url", though it gives nice features to virtual keyboard users.
  • The URL become difficult to manipulate on dev side. They needed to be tweaked in several way to detect Pop-Up and insert term.
  • It's a very rigid system. With the official LWT, a third feature "word encoding" was present and was incredibly hard to implement.

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:

  • Replace term placeholder ### by lute_term.
  • Replace the placeholder indicator * by an argument lute_popup=1
  • It will easily be extended on the fly.

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
Copy link
Owner

jzohrab commented Apr 13, 2023

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:

  • dictionary URL : textbox, the url with "###" placeholders :-D
  • opens in pop-up? : checkbox
  • encoding : dropdown or textbox
  • returns : dropdown (html default, or json -- reason for the "json" option is that some languages seem to only have dictionaries available via a json API)

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
Copy link
Contributor Author

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 😄

@jzohrab
Copy link
Owner

jzohrab commented Apr 15, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

jzohrab commented Apr 16, 2023

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
Copy link
Contributor Author

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 😄 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.

@HugoFara HugoFara closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants