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

po/lua submodule #16135

Merged
merged 2 commits into from
Sep 8, 2024
Merged

po/lua submodule #16135

merged 2 commits into from
Sep 8, 2024

Conversation

TurboGit
Copy link
Member

This is a POC for a PR made on the Lua script repository to support Lua script translation using dt support.

@wpferguson : This is the dt part for make the Lua script PR working:

See darktable-org/lua-scripts#450

@TurboGit TurboGit added wip pull request in making, tests and feedback needed lua labels Jan 17, 2024
@TurboGit TurboGit added this to the 4.8 milestone Jan 17, 2024
@TurboGit TurboGit requested a review from wpferguson January 17, 2024 11:37
@TurboGit TurboGit self-assigned this Jan 17, 2024
@TurboGit TurboGit marked this pull request as draft January 17, 2024 11:38
@wpferguson
Copy link
Member

This might actually be easy...

I added a locale_dir to the darktable.configuration table (attached lua/src/configuation.c).

configuration.c.txt

@TurboGit
Copy link
Member Author

I have committed your new configuration.c in this PR.

@TurboGit TurboGit modified the milestones: 4.8, 5.0 May 13, 2024
@TurboGit TurboGit changed the title POC: po/lua submodule po/lua submodule Aug 21, 2024
@TurboGit TurboGit marked this pull request as ready for review August 21, 2024 19:21
@TurboGit
Copy link
Member Author

@wpferguson : Can you double check all is there?

Should we translate all Lua scripts or start with the official ones? For the record this is around 700 more strings, quite lot of work ahead :)

This makes the Lua scripts first-class citizen in darktable.
@TurboGit
Copy link
Member Author

I have checked the strings and see some that may not to be translated. Can you check:

  • identity_file_chooser
  • export_location_chooser
  • "" (empty string to be translated) ( dt.new_widget("section_label"){label = _(" ")}, script_manager line 1355)
  • We have many tags like $(EXIF_YEAR) - EXIF year but on dt the separator between EXIF & YEAR is a dot not a '_'. Would be nice to be consistent avoiding quite some translation.
  • $(SECOND) - current second is on dt $(SECOND) - second again better to keep consistent (likewise for some other tags)
  • export_pdf
  • We have latitude: and latitude:
  • Likewise for longitude
  • We have at least 3 strings starting with geoJSON export: , maybe this sting can be shared
  • ...

And more I think about this I'd probably prefer starting with officials scripts for now. What do you think? And add others later (but before 5.0), this will be a faster path to something usable that will get some field testing.

@wpferguson
Copy link
Member

I agree, let's start with official.

I'll work on the list above.

@TurboGit
Copy link
Member Author

TurboGit commented Sep 7, 2024

@wpferguson : What's the status of this? I don't want to put too much pressure, but since this will bring quite a lot of new strings I'd like to have this merged if possible in September. TIA.

@wpferguson
Copy link
Member

I got the official scripts ready for translation and merged PR (above) into the lua-scripts.

I created an API-9.3.0 branch so that the merged scripts wouldn't affect 4.8.1 (script_manager checks the API against the repository branches and checks out the correct one)

After I had fixed the official scripts I thought you were going to merge and let the translators get started on them. Maybe I misunderstood. I started working on the example scripts, but I hadn't finished them yet because I didn't want to add more messages to be translated before work had started on the official scripts.

I looked at the list and I have fixes for all of them, but again I didn't move forward because work hadn't started on the official scripts. My plan is to do the example scripts, then tools, then contrib in 2 or 3 batches.

One note, translations can't be tested until #17300 is merged as the current API is 9.3.0 and script_manager will check out that branch of the lua-scripts instead of using the master branch. #17300 will bump the API to 9.4.0.

@TurboGit
Copy link
Member Author

TurboGit commented Sep 8, 2024

Maybe I'm the one confused now :)

You said:

I'll work on the list above.

But since I do not follow closely the lua repository I don't know if it is ready or not...

Anyway, I kind of understand that you want me to merge this PR now, right? Sorry for all my stupid questions, I'm not clear on this...

@wpferguson
Copy link
Member

I kind of understand that you want me to merge this PR now, right?

Yes please.

I do not follow closely the lua repository I don't know if it is ready or not

I will tag you on commits and give you my thoughts on where we stand.

My plan going forward:

  • fix the stuff on the list
  • get the example scripts ready
  • get the tools scripts ready
  • get the contrib scripts ready in 2 or 3 batches.

@TurboGit
Copy link
Member Author

TurboGit commented Sep 8, 2024

Ok, I'm merging now but then the list above must be fixed ASAP to ensure that translation won't be invalidated, there is still a lot to translate better avoiding asking our translators too much work :)

@TurboGit TurboGit merged commit 4b6e0d1 into darktable-org:master Sep 8, 2024
7 checks passed
@wpferguson
Copy link
Member

@TurboGit

We have many tags like $(EXIF_YEAR) - EXIF year but on dt the separator between EXIF & YEAR is a dot not a '_'. Would be nice to be consistent avoiding quite some translation.

I propose turning this, _("$(EXIF_FOCUS_DISTANCE) - EXIF focus distance\n") into string.format("$(EXIF_FOCUS_DISTANCE) - EXIF %s\n", _("focus distance"))

The underscore notation is in contrib/rename_images.lua but all the variable substitution code will be replaced with the string library functions that do the same and use the . notation.

@TurboGit
Copy link
Member Author

BTW @wpferguson let me know when your work is over so I can let translators start working on this huge number of new strings. TIA.

@wpferguson
Copy link
Member

@TurboGit I'll do that. Right now I'm working on the last item from the list, variable substitution.

@TurboGit
Copy link
Member Author

@wpferguson : Thanks! As we are at it, to integrate the module better I would propose a renaming of "script manager". All current module name are simple nouns : Collections, History stack... So why not have just "Scripts" ? Or something like that. I had thought about "Lua scripts", but well end users don't care about it being Lua or whatever and they don't even know what is Lua. So "Scripts" looks simple, we could alternatively have "External scripts".

What do you think?

@wpferguson
Copy link
Member

In #17422 I jokingly suggested calling it Features to get past some user's ideas that scripts are hacks or not "real" code.

I don't have a strong attachment to the name script_manager, so scripts is fine.

@victoryforce
Copy link
Collaborator

So why not have just "Scripts" ? Or something like that.

Another idea is "Extensions". We are emphasizing not that these are "scripts" (in the sense of not "real code"), but that they are extensions that we can activate.

@wpferguson
Copy link
Member

@victoryforce I thought of that right after I finished the above comment 😄

@TurboGit
Copy link
Member Author

I have a slight preference for Scripts over Extensions as script seems common (used by GIMP for example). No strong opinion...

@wpferguson
Copy link
Member

wpferguson commented Sep 10, 2024

and you want the first letter capitalized? Or should it be lower case like the rest of the modules?

You can play with it by editing tools/script_manager.lua and changing the second argument to dt.register_lib (line 1168). Then you can see what it looks like and decide

@TurboGit
Copy link
Member Author

@wpferguson : Lower case of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lua wip pull request in making, tests and feedback needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants