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.

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

Parameters for File formats #1897

Open
BhaaLseN opened this issue Mar 11, 2018 · 11 comments
Open

Parameters for File formats #1897

BhaaLseN opened this issue Mar 11, 2018 · 11 comments
Assignees
Labels
enhancement Adding or requesting a new feature.

Comments

@BhaaLseN
Copy link

BhaaLseN commented Mar 11, 2018

There are a few file formats in translate-toolkit that accept parameters and/or allow for customization, for example the JSON format which at this point has pre-commit hooks to change indentation etc.
I'm also working on a XML format that would currently require hardcoding certain parameters and duplicating the FileFormat/FileUnit (and to some degree even the translate-toolkit TranslationStore depending on the kind of parameter/customization) for every specialization.

It would be nice if the FileFormat could expose optional/required parameters as list/map, which would then be presented to the admin when configuring a component (by showing/hiding new textboxes below the format combobox). Internally they could either be passed/unpacked into the FileFormat.loader.__init__ or pass through a dedicated factory method in case __init__ is not enough.

There is one caveat I can think of: Autodetection wouldn't really work depending on the kind of parameter/customization. But to be fair, I never used autodetection (primarily because I know what format it should be; but also because it may not find a unique/correct match in some cases); so this might be a non-issue.

@nijel
Copy link
Member

nijel commented Mar 12, 2018

In (upcoming) Weblate 2.20, this is handled through addons:

screenshot-2018-3-12 languagetool browser addon

This is probably not perfect place, but was easiest to integrate.

PS: For JSON this depends on translate/translate#3772 which is not yet in any released translate-toolkit version.

@nijel nijel added enhancement Adding or requesting a new feature. undecided These features might not be implemented. Can be prioritized by sponsorship. labels Mar 12, 2018
@BhaaLseN
Copy link
Author

BhaaLseN commented Mar 12, 2018

Nice! You've commented on translate/translate#3776 so I suppose you also saw the __init__ methods there. If addons allow me to customize those, I can totally live with that!

@nijel
Copy link
Member

nijel commented Mar 12, 2018

Currently it's rather post init where the settings are applied (because this is way it's implemented for current formats as that only controls saving and not load).

The problem for passing args to __init__ there is chicken-egg problem: the addon can be configured only after component is created, so it really can not influence settings for first loading...

@BhaaLseN
Copy link
Author

Hm, that would be a little difficult, especially since I have a few cases where I want to force one specific element name but ignore (and also preserve!) others - so I cannot really go and do some parameter autodetection during load (which might be wrong) and change them later on.
That would also mean my flat xml format is basically useless at this point (unless they go and define a copy of the format inside Weblate with the right parameters).

I can personally live with that for my own on-premise installation, but hosted Weblate users won't have that luxury.

But...what about receiving back a map from a pre-init event and unwrapping it into self.subproject.file_format_cls.parse? All before the actual store is created.
Not sure if that is good enough though; while looking at other files I noticed at least three calls to parse scattered all around...which might as well require that to work properly.

@nijel
Copy link
Member

nijel commented Mar 12, 2018

Needing configuration at parse time is maybe good reason to revisit decision to doing such configuration in addons and do that really on the file format layer instead. The problem is how to integrate that into Django admin interface when creating new components...

@BhaaLseN
Copy link
Author

Did some testing and the Django admin interface is really...well, static. I've seen blog posts and tutorials that replace the full page with a custom one and then write JavaScript and all that; but that feels a little overkill.

One other thing I did try out was to write an addon for my derived class (which I modified to not pass the file name so the format would do a blank but remember the filename for later); then change the fields and call a custom parse method (to effectively defer the load until after the addon did its work).
Turns out it wouldn't call it for existing files...or I made a mistake in my addon.

So, not too sure if (ab?)using addons as basically a factory is a good idea. Plus it probably still runs into the chicken-egg problem since it is configured after the component (with all files and stuff) is created.

Another idea floating in my head would be to treat the current file formats as templates, types or whatever (that don't perform the work just yet) and let the user first configure instances of those formats (including parameters where applicable/necessary). Those preconfigured instances are then selected when creating/editing a component. This would also cut down the number of formats in the list in case not all of them are used (which is most certainly the case in my installation; where I only need 3 formats).
But it might still run into the same Django admin interface issue...

@nijel
Copy link
Member

nijel commented Mar 14, 2018

In the long term I want to get rid of the Django admin interface (at least for usual operations), but that's still far in the future...

Maybe this configuration could be pushed to settings.py, in similar way as template loaders are configured? That way passing params is possible, though most of the formats won't need one.

@BhaaLseN
Copy link
Author

Thats another option for me (just like duplicating the format); but this still leaves hosted users dead in the water (or can they modify their own settings.py there?)
Not too familiar with what hosted users can and cannot do to their instance.

I can totally live with a long-term goal too though; since I have various near-term workarounds to get it working on my on-permise installation.
In the mean time, #1820 would make my life easier during updates though ;)

@BhaaLseN BhaaLseN mentioned this issue Apr 23, 2018
4 tasks
nijel added a commit that referenced this issue Nov 14, 2019
Instead of using parsefile from translate-toolkit, skip some indirection
and call directly parse. This avoids some not needed complexity in the
translate-toolkit and it will also allow us to customize the class prior
to parsing (see #1897).

Signed-off-by: Michal Čihař <[email protected]>
nijel added a commit that referenced this issue Nov 14, 2019
This should be safer mode of operation and allows us to customize loading
as well (see #1897).

Signed-off-by: Michal Čihař <[email protected]>
@nijel
Copy link
Member

nijel commented Nov 14, 2019

Revisiting this again, as some customization is needed in more cases:

Some of these can be handled with addons, but that's not good place - it can't set the parameters on the initial import. Also, doing that per component is not optimal, as for typical installation same format will be used on multiple components.

Therefore what we need is customizable list of file formats. That gives two options:

  • Define formats in the settings.py, extending current syntax to allow defining parameters for the formats
  • Define formats in the database, making it editable in the management interface

First option is easier to implement, but might be harder to use. On the other side this is something what will be done few times, so it might be not worth of implementing UI for that.

@BhaaLseN
Copy link
Author

BhaaLseN commented Nov 15, 2019

I'm totally fine with the first option. I already limited the list of available formats to the ones we use (which is like, 3 instead of like 20 or so?) in my settings.py anyways to keep the list short when we add new projects and/or components.
Right now I have my own classes living in weblate/customizations/formats.py for Flat XML that just hardcode the names we use; which are then also added to settings.py in WEBLATE_FORMATS.

One reason to go for the second option is probably that the first one only works for self-hosted?

@nijel
Copy link
Member

nijel commented Jul 22, 2020

I think we really need to replace file-format addons by proper customization of them. Possible approach:

  • Add file_format_params JSON field to the Component model
  • Add customization form fields to all component forms
    • Each parameter will have a separate field
    • JavaScript will toggle which of them are visible
  • Upon save, all applicable custom fields are serialized into file_format_params
  • The parse method of formats will accept these as params arg
  • Add places which copy component params should copy file format params as well
  • Discovery addons needs to support this

This was referenced Jul 22, 2020
nijel added a commit that referenced this issue Nov 19, 2020
Otherwise the addon configuration is not really applied here.

This is really a workaround until we move all these to file
format configuration, see #1897
@nijel nijel removed the undecided These features might not be implemented. Can be prioritized by sponsorship. label Nov 19, 2020
@nijel nijel added this to the 4.5 milestone Nov 19, 2020
@nijel nijel removed this from the 4.5 milestone Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding or requesting a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants