-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix full/screen capture use last region #3364
Fix full/screen capture use last region #3364
Conversation
Thanks for the PR. I believe instead of "exclusion" criteria we should only limit this to |
Thanks for the feedback. That makes a lot of sense. |
Yes, you right, the current wording is sub-optimal. Would you like to also include those changes in this PR (considering that it kinda fits the context)? What needs to be changed is the config window text and it's tooltip. |
I am not sure how the translation system works, will changing the keys invalidate all current translations? I can include it in this PR, but the translations should be a separate issue I suppose. |
Okay, after some digging I believe here is what I need to do.
|
As far as I understand, you don't need to care about translations. All you need to do is to change the code for English keys. Then weblate will pull changes from the master branch and automatically roll it as a string to be translated in all the supported languages. This is also my understanding. I'm relatively new to handling translation conflicts (already a messy one has happened and I'm in the process of merging the translations manually between the master and the Weblate fork). For now, I have locked the Weblate from the admin dashboard, so it is safe change anything on our side. |
Just tried changing English in code and compile with the Btw, thanks a lot for the translation merging work, I definitely also noticed this messy situation. |
Looks like this will be merged in the current state. |
@Kit-p I've reviewed the patch file and it looks good. Although I can't apply it for some reason - it says I think you can commit the changes from the patch here, and we'll merge them all together. But let's wait for @mmahmoudian to confirm. @mmahmoudian You mentioned a messy merge, are you talking about the Internationalization_ta.ts file between current weblate/master and master? If yes, it looks like that file effectively has no changes, but the whole file shows up as changed in the diff viewer. This is probably due to a change in windows/linux line endings. There are also some other conflicts, but not many and not especially tricky, unless I'm looking at the wrong thing. |
I think the file in question was
If you have the setup ready for merging please go ahead and merge. I haven't had time this week to work on it. If not, I will try to merge tomorrow. |
@veracioux Ready for review, thanks! |
@Kit-p Merged. Thank you for your contribution! |
* Fix full capture use last region * Limit use last region to gui capture only * Update config use last region wordings * Preserve current translations for config use last region wordings (cherry picked from commit f5f2e53)
Ignore "Use last region" option when running
flameshot full
orflameshot screen
.Closes #3086.