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

feat: change the writable glossary by a right click #1108

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Gen-Rainbow
Copy link

@Gen-Rainbow Gen-Rainbow commented Aug 18, 2024

Add a new feature. Now users can change the writable glossary by right clicking the editor window. (When the glossary folder has more than one files)
屏幕截图 2024-08-22 091042

Pull request type

  • Feature enhancement -> [enhancement]

Which ticket is resolved?

What does this PR change?

  • src/org/omegat/gui/editor/EditorPopups.java
  • src/org/omegat/Bundle.properties
  • src/org/omegat/Bundle_zh_CN.properties

…megat-org#1003 on SourceForge. Now users can change the writable glossary by right clicking the editor window.
Copy link
Member

@miurahr miurahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several coding style issues. Please check my comments.

src/org/omegat/gui/editor/EditorPopups.java Outdated Show resolved Hide resolved
src/org/omegat/gui/editor/EditorPopups.java Outdated Show resolved Hide resolved
src/org/omegat/gui/editor/EditorPopups.java Outdated Show resolved Hide resolved
@miurahr
Copy link
Member

miurahr commented Aug 19, 2024

Warning: [ant:checkstyle] [WARN] /home/runner/work/omegat/omegat/src/org/omegat/gui/editor/EditorPopups.java:310: Line is longer than 120 characters (found 127). [LineLength]
Warning: [ant:checkstyle] [WARN] /home/runner/work/omegat/omegat/src/org/omegat/gui/editor/EditorPopups.java:319: Line is longer than 120 characters (found 122). [LineLength]
Warning: [ant:checkstyle] [WARN] /home/runner/work/omegat/omegat/src/org/omegat/gui/editor/EditorPopups.java:400: Line is longer than 120 characters (found 122). [LineLength]
Warning: [ant:checkstyle] [WARN] /home/runner/work/omega/omegat/src/org/omegat/gui/editor/EditorPopups.java:437: Line is longer than 120 characters (found 121). [LineLength]

There are also several checkstyle warnings in the change. Spotless will automatically format the code.

@brandelune
Copy link

The SF links do not seem to work. Please edit them.

@brandelune
Copy link

Please describe how your feature works. Add screenshots and precise descriptions please.

Copy link
Member

@miurahr miurahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for update the coding style. Here is a proposal to improve comment.

src/org/omegat/gui/editor/EditorPopups.java Outdated Show resolved Hide resolved
Copy link
Member

@miurahr miurahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When switching writable glossary file, and adding a term to there in a team project, remote glossary is not updated. And when reloading, or close and open the project, the term, added by the feature, is gone. The local glossary file is over-write with a remote one.

When adding default glossary file, there is no problem.

Please fix the problem if you claim to add a feature to update non-default glossary, it also should work also with team projects.

As a definition of OmegaT team project, non-standard glossary files are managed by team administrator and distributed by admin.
If you add a feature to allow users, not admin, to manipulate non-default glossaries,
you also need to design some mechanisms for a team project feature.

@Gen-Rainbow
Copy link
Author

When switching writable glossary file, and adding a term to there in a team project, remote glossary is not updated. And when reloading, or close and open the project, the term, added by the feature, is gone. The local glossary file is over-write with a remote one.

When adding default glossary file, there is no problem.

Please fix the problem if you claim to add a feature to update non-default glossary, it also should work also with team projects.

As a definition of OmegaT team project, non-standard glossary files are managed by team administrator and distributed by admin. If you add a feature to allow users, not admin, to manipulate non-default glossaries, you also need to design some mechanisms for a team project feature.

Well, that's something new to me. May need some time to figure it out.

@Gen-Rainbow
Copy link
Author

When switching writable glossary file, and adding a term to there in a team project, remote glossary is not updated. And when reloading, or close and open the project, the term, added by the feature, is gone. The local glossary file is over-write with a remote one.

When adding default glossary file, there is no problem.

Please fix the problem if you claim to add a feature to update non-default glossary, it also should work also with team projects.

As a definition of OmegaT team project, non-standard glossary files are managed by team administrator and distributed by admin. If you add a feature to allow users, not admin, to manipulate non-default glossaries, you also need to design some mechanisms for a team project feature.

I use git as my versioning tool and I have run a few tests. Everything worked well. Both me and my team members tried updating the non-standard glossary files and saving them (ctrl+S) and reloading the project (F5), they didn't just gone. The entries stay in the files which they belong to.

It doesn't make sense if the default glossary file should work and non-default fail, since my feature doesn't change anything like transmission with remote repository. Please check if you have added the non-default glossary file in your versioning system or if you properly saved and reloaded the project.

In terms of the local file overwritten by a remote one, that just happen when a member committed a newer file and I reloaded the project. I don't think it can be avoided. If someone feels uncertain about the changes in non-standard files or anything in a team project, my advice is to open a new branch for every team member. Then changes can be committed in form like pull requests.

Please let me know if anything goes wrong!

@miurahr
Copy link
Member

miurahr commented Aug 26, 2024

I use git as my versioning tool and I have run a few tests. Everything worked well. Both me and my team members tried updating the non-standard glossary files and saving them (ctrl+S) and reloading the project (F5), they didn't just gone. The entries stay in the files which they belong to.

"reloading the project(F5)" does not load remote content but reload only local content in OmegaT 6.0.0.
OmegaT 6.1.0, a master branch, it reloads contents from the remote.

I close the project. and open it again when I observe it.

@miurahr
Copy link
Member

miurahr commented Aug 26, 2024

Your change does not update a value of project properties of writable glossary path.
Your UI tells "change writable glossary", it is not consistent.

It is because OmegaT project has a writeable glossary property.

As in defined in ProjectUICommand class, a writable glossary file is defined as like

    public static void openWriteableGlossaryFile(boolean parent) {
        if (!Core.getProject().isProjectLoaded()) {
            return;
        }
        String path = Core.getProject().getProjectProperties().getWriteableGlossary();
        if (StringUtil.isEmpty(path)) {
            return;
        }
        File toOpen = new File(path);
        if (parent) {
            toOpen = toOpen.getParentFile();
        }
        openFile(toOpen);
    }

The method is called when you access OmegaT menu "Project" > "Access Project Files" > "Access Writeable Glossary".
You may be interested in ProjectProperties#setWriteableGlossary method.

@Gen-Rainbow
Copy link
Author

About the problem you pointed out, I think I might get it.

It seems that OmegaT will detect the writable glossary file when updating the project. If any changes were in the writable glossary file, it will be committed to the remote. In other words, changes in any other non-writable glossary file will just be ignored.

@Gen-Rainbow
Copy link
Author

So if we want any changes in a non-default glossary file committed to the remote, that specific glossary file need to be set as the 'writable' glossary file.

@Gen-Rainbow
Copy link
Author

Your change does not update a value of project properties of writable glossary path.
Your UI tells "change writable glossary", it is not consistent.

Please check the project properties (ctrl+E) again when you change the writable glossary. It should have been altered.

@Gen-Rainbow
Copy link
Author

You may consider this feature as a shortcut to open the project props dialog then browser and change it manually (without reloading the project).

item.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
props.setWriteableGlossary(writableGlossPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I find it.

@brandelune
Copy link

So if we want any changes in a non-default glossary file committed to the remote, that specific glossary file need to be set as the 'writable' glossary file.

I’m not sure that’s something we want to do.

The writeable glossary is the shared glossary.
Specific glossaries do not have to be shared glossaries. If fact, specific glossaries have good reasons to not be shared glossaries.

So before you go any further, I’d like you to precisely define what you want to have so that we can discuss what is good for OmegaT.

Thank you for your understanding.
Jean-Christophe
(OmegaT project manager)

@Gen-Rainbow
Copy link
Author

I’m not sure that’s something we want to do.

The writeable glossary is the shared glossary. Specific glossaries do not have to be shared glossaries. If fact, specific glossaries have good reasons to not be shared glossaries.

So before you go any further, I’d like you to precisely define what you want to have so that we can discuss what is good for OmegaT.

Thank you for your understanding. Jean-Christophe (OmegaT project manager)

I see. To be honest, I initially designed this feature because I encountered some inconvenience while using OmegaT alone. When I visited the OmegaT official website for help, I found that some users had the same problem as me. So I didn't consider the issue of team sharing, but directly created this feature.

I think maybe we can add a switch or something in the preference which give the right of turning on/off this feature to the users. I just tried to offer a different option.

For individual translators like myself, we can make use of this function. While translators work as a team can also just ignore it and don't need to change anything. It's all up to the users, all up to OmegaT project team.

@brandelune
Copy link

I’m not sure that’s something we want to do.
The writeable glossary is the shared glossary. Specific glossaries do not have to be shared glossaries. If fact, specific glossaries have good reasons to not be shared glossaries.
So before you go any further, I’d like you to precisely define what you want to have so that we can discuss what is good for OmegaT.
Thank you for your understanding. Jean-Christophe (OmegaT project manager)

I see. To be honest, I initially designed this feature because I encountered some inconvenience while using OmegaT alone. When I visited the OmegaT official website for help, I found that some users had the same problem as me. So I didn't consider the issue of team sharing, but directly created this feature.

I think maybe we can add a switch or something in the preference which give the right of turning on/off this feature to the users. I just tried to offer a different option.

For individual translators like myself, we can make use of this function. While translators work as a team can also just ignore it and don't need to change anything. It's all up to the users, all up to OmegaT project team.

:-) It’s up to you to make a proposal that makes sense with your practice of OmegaT. And I’m really not in favor of switches here and there.

So, what I’m asking, now that you see that we have some constraints, is just to write down your use case, a "textual" mock-up of the user interface, so that we can discuss the details of the "general" user experience.

It is fine to have default behaviors that do not come with a battery of options. We just need to be careful about the defaults we propose.

And thank you for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants