10984:Improve TagsField of keywords #11853

MrunalKashid02 · 2024-09-29T16:37:13Z

Fixes #10984

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Siedlerchr · 2024-09-29T19:14:52Z

You need to merge upstream/main in your branch and resolve the merge conflicts

MrunalKashid02 · 2024-09-30T17:44:34Z

Merge conflicts because draft PR? or else

Siedlerchr · 2024-09-30T18:04:14Z

Merge conflicts happen when the same code was edited by different people:
https://www.atlassian.com/git/tutorials/using-branches/merge-conflicts

You need to merge the upstream/main into your branch and then you can use Intellij to resolve them:
https://www.jetbrains.com/help/idea/resolve-conflicts.html

subhramit

Hi Mrunal. May I ask which AI you used to generate the code?

MrunalKashid02 · 2024-10-01T02:52:43Z

Is there anything wrong?

subhramit · 2024-10-01T03:02:42Z

Is there anything wrong?

Not with the fact that you considered contributing, but we don't appreciate it when PRs submitted have majority of the code generated by AI. It increases our effort to review and the candidate's effort to gather context about their own code and find fixes.

At JabRef, we strive to provide valuable learning experiences to contributors to help them grow their skills. The team invests significant time and resources, with a 2-to-1 ratio of senior developers mentoring each student. When a contribution lacks genuine effort, it not only affects maintainers' time but also takes away opportunities from other students who are eager to learn and contribute meaningfully.

MrunalKashid02 · 2024-10-01T03:15:00Z

I apologise for my mistake. will be ok if I again start by but it requires some time for each task because this is my first time for contribution. Once again sorry.

MrunalKashid02 · 2024-10-01T03:16:33Z

Please can if you give me some time to work on this issue.

subhramit · 2024-10-01T03:17:44Z

I apologise for my mistake. will be ok if I again start by but it requires some time for each task because this is my first time for contribution. Once again sorry.

It's alright. Try to understand what the code in your PR does properly and see if you can continue. If you are not confident about finishing it, you can ask to be unassigned and pick smaller issues to start with.

MrunalKashid02 · 2024-10-01T03:28:24Z

There are 1-4 task. I can do each task one by one this are small task I guess. if you feel that this issue is resolved quickly you may unassigned me. I really appreciate guidance and support.

subhramit · 2024-10-01T03:32:22Z

There are 1-4 task. I can do each task one by one this are small task I guess. if you feel that this issue is resolved quickly you may unassigned me. I really appreciate guidance and support.

If you can complete any sub-task of the issue fully, that's also fine.

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

You can check review dog's comments at the tab "Files changed" of your pull request.

Siedlerchr · 2024-10-07T17:00:28Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

        Bindings.bindContentBidirectional(keywordTagsField.getTags(), viewModel.keywordListProperty());
    }

+    private void handlePasteAction() {
+        Clipboard clipboard = Clipboard.getSystemClipboard();


Use JabRef's ClipBoardManager with getcontents

Siedlerchr · 2024-10-07T17:00:56Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

+                String[] keywords = pastedText.split(String.valueOf(viewModel.getKeywordSeparator()));
+
+                for (String keywordText : keywords) {
+                    Keyword keyword = viewModel.getStringConverter().fromString(keywordText.trim());


Use Keyword.of

Siedlerchr · 2024-10-07T17:06:57Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

+
+                    // Add keyword if it's valid and not already present
+                    if (keyword != null && !keywordTagsField.getTags().contains(keyword)) {
+                        keywordTagsField.getTags().add(keyword);


You should not operate here on the keywordTagsField directly, instead use the viewModel.keywordListProperty
and simply call add (it already performs a contains check)

ok i will make changes

Siedlerchr · 2024-10-15T18:41:14Z

@MrunalKashid02 How is the status here? Would be great if you could address the comments and finish this

MrunalKashid02 · 2024-10-15T19:15:38Z

I am remain with some changes . I will commit changes tomorrow. Will it fine ?

koppor · 2024-10-16T08:15:37Z

I am remain with some changes . I will commit changes tomorrow. Will it fine ?

Sure. We just need to know whether there is any more activity intended. Otherwise, we will close the PR and look for another contributor.

koppor · 2024-10-16T19:54:11Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

+            String pastedText = ClipBoardManager.getContents();
+
+            if (!pastedText.isEmpty()) {
+                String[] keywords = pastedText.split(String.valueOf(viewModel.getKeywordSeparator()));


Please use org.jabref.model.entry.KeywordList

If it is not directly possible, because hierarchicalDelimiter is missing, use > as hierarchicalDelimiter.

Can elobarate more how this can be done

koppor · 2024-10-16T19:55:41Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

@@ -86,16 +93,84 @@ public KeywordsEditor(Field field,
                event.consume();
            }
        });
+        keywordTagsField.getEditor().setOnKeyPressed(event -> {
+            if (event.isControlDown() && event.getCode() == KeyCode.V) {


this does not respect org.jabref.gui.keyboard.KeyBinding#PASTE. Please look at other codes how keyboard shortcuts are handled.

koppor · 2024-10-16T19:56:03Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

@@ -86,16 +93,84 @@ public KeywordsEditor(Field field,
                event.consume();
            }
        });
+        keywordTagsField.getEditor().setOnKeyPressed(event -> {
+            if (event.isControlDown() && event.getCode() == KeyCode.V) {
+                handlePasteAction();  // Call paste handler


Remove code comments just repeating the method name. (This is an indication of AI-generated code...)

Yeah i made mistake previous by use ai. but the changes i am doing by understanding codebase and as per suggestion i didnt take help from ai for code. I will make changes.

koppor · 2024-10-16T19:58:32Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

+        });
+        keywordTagsField.getEditor().setOnContextMenuRequested(event -> {
+            ContextMenu contextMenu = new ContextMenu();
+            MenuItem pasteItem = new MenuItem("Paste");


Non-Localized. Please read at https://devdocs.jabref.org/code-howtos/localization.html how to Localize.

koppor · 2024-10-16T19:58:55Z

src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java

+            }
+        });
+        keywordTagsField.getEditor().setOnContextMenuRequested(event -> {
+            ContextMenu contextMenu = new ContextMenu();


This should be a class variable an initalized there?

MrunalKashid02 · 2024-10-22T06:16:18Z

I am getting this on mail what does it mean

koppor · 2024-10-27T00:52:27Z

I am getting this on mail what does it mean

That you forked JabRef and "our" workflows are running at your side, too.

I tried to disable them for forks (also for you then) - Limit some workflows to main repository #12097
I field an upstream issue Feature: Retry on rate limit takanome-dev/assign-issue-action#251

You need to update the main branch in your fork, then the error should be gone!

subhramit · 2024-11-07T12:19:54Z

@MrunalKashid02 hey, are you still working on it? You can otherwise close the PR.

MrunalKashid02 · 2024-11-07T13:04:13Z

Yeah I am busy bcoz of exam I will update you tonight.

10984:Improve TagsField of keywords

a7f11ce

MrunalKashid02 mentioned this pull request Sep 29, 2024

Improve TagsField of keywords #10984

Open

subhramit reviewed Oct 1, 2024

View reviewed changes

Merge branch 'main' into issue-10984-improve-tagfield-keywords

c324c35

github-actions bot reviewed Oct 4, 2024

View reviewed changes

LoayGhreeb added 2 commits October 5, 2024 05:44

Merge branch 'main' into pr/11853

6830f0b

Update KeywordsEditor.java

65a9cbe

MrunalKashid02 marked this pull request as ready for review October 5, 2024 03:26

Siedlerchr reviewed Oct 7, 2024

View reviewed changes

Siedlerchr requested changes Oct 7, 2024

View reviewed changes

Siedlerchr added the status: changes required Pull requests that are not yet complete label Oct 7, 2024

Done Required Changes suggested

3ce227e

koppor requested changes Oct 16, 2024

View reviewed changes

MrunalKashid02 closed this Nov 10, 2024

koppor added the keywords label Nov 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

10984:Improve TagsField of keywords #11853

10984:Improve TagsField of keywords #11853

MrunalKashid02 commented Sep 29, 2024 •

edited

Loading

Siedlerchr commented Sep 29, 2024

MrunalKashid02 commented Sep 30, 2024

Siedlerchr commented Sep 30, 2024

subhramit left a comment

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

github-actions bot left a comment

Siedlerchr Oct 7, 2024

Siedlerchr Oct 7, 2024

Siedlerchr Oct 7, 2024

MrunalKashid02 Oct 7, 2024

Siedlerchr commented Oct 15, 2024

MrunalKashid02 commented Oct 15, 2024

koppor commented Oct 16, 2024

koppor Oct 16, 2024

MrunalKashid02 Nov 9, 2024

koppor Oct 16, 2024

koppor Oct 16, 2024

MrunalKashid02 Oct 19, 2024

koppor Oct 16, 2024

koppor Oct 16, 2024

MrunalKashid02 commented Oct 22, 2024

koppor commented Oct 27, 2024

subhramit commented Nov 7, 2024

MrunalKashid02 commented Nov 7, 2024

10984:Improve TagsField of keywords #11853

10984:Improve TagsField of keywords #11853

Conversation

MrunalKashid02 commented Sep 29, 2024 • edited Loading

Mandatory checks

Siedlerchr commented Sep 29, 2024

MrunalKashid02 commented Sep 30, 2024

Siedlerchr commented Sep 30, 2024

subhramit left a comment

Choose a reason for hiding this comment

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

MrunalKashid02 commented Oct 1, 2024

subhramit commented Oct 1, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Siedlerchr commented Oct 15, 2024

MrunalKashid02 commented Oct 15, 2024

koppor commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrunalKashid02 commented Oct 22, 2024

koppor commented Oct 27, 2024

subhramit commented Nov 7, 2024

MrunalKashid02 commented Nov 7, 2024

MrunalKashid02 commented Sep 29, 2024 •

edited

Loading