Remove second "Citation key" column from the Consistency check results table. by Paras-Gupta16 · Pull Request #12699 · JabRef/jabref

Paras-Gupta16 · 2025-03-12T00:56:00Z

This PR remove the second "Citation key" column from the Consistency check results table.

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
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.

github-actions

Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues.
In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

koppor · 2025-03-12T01:12:26Z

+
+        removeColumnByTitle(InternalField.KEY_FIELD.getName());


At least, there should be a Java comment why this can be removed. (You can say that this is already displayed in the first column. But then, you need to somehow point in the coude where it is added)

Maybe, this column should be removed from the viewModel in the first place?

Yes, this is more of a hotfix. @Paras-Gupta16 please investigate the cause and fix the bug there.

On debugging the viewmodel class the getColumnNames() method returns a string. This can be done by deleting the column by manipulating the string.

Another thing is code is already written here.

I took a look. Some hints:
The function you mention returns a list of strings. If by "deleting the column by manipulating the string" you mean removing result.add("Citation Key");, that will not work as it will remove the first Citation key column. One way is either making the second one functional and removing the first, or keeping the first and putting a filter such that in the section:

allReportedFields.forEach(field -> { result.add(field.getDisplayName()); });

field.getDisplayName() is added only if it is not equal to Citation Key.
Not letting the column be generated is better than generating it and then removing it - the latter is more expensive - so the current solution has to be changed to one of the above.
What do you think would be better? @priyanshu16095

Not letting the column be generated is better than generating it and then removing it - the latter is more expensive - so the current solution has to be changed to one of the above.

Yeah, I also learned from this point.
Firstly, as he commented, "the code is already there," I thought this was more appropriate.

A simple check can be added, and tableData and writeBibEntry should also be checked.

Removed it in class before creating the table.

Why is this line still needed then?

koppor · 2025-03-12T01:13:36Z

@priyanshu16095 Can you review, too, please?

priyanshu16095 · 2025-03-12T01:14:31Z

Sure, I will.

priyanshu16095 · 2025-03-12T02:32:43Z

I have a thought after this task: the results from the GUI and CLI will be different? Should this also be done for the CLI

koppor · 2025-03-12T08:22:56Z

I have a thought after this task: the results from the GUI and CLI will be different? Should this also be done for the CLI

If it is about the citatoin key column removal: Yes, please. Then, the CLI is also wrong. -- Which means, the input data is wrong?

Regarding the other columns (#12696), my thoughts are (but you haven'T ask for it)

The CLI has a complete table of all types - and one cannot filter.

The GUI shows a single type - and not all types together. The GUI should be compact. Thus, removing columns is OK there.

priyanshu16095 · 2025-03-14T18:17:15Z

-        });
-        return result;
+        allReportedFields.forEach(field-> result.add(field.getDisplayName().trim()));
+        return new ArrayList<>(result);


The new ArrayList should not be returned. Modify the methods to use the set instead.

Also fix the check.

priyanshu16095 · 2025-03-14T22:34:01Z

+    public Set<String> getColumnNames() {
+        Set<String> result = new LinkedHashSet<>(columnCount + EXTRA_COLUMNS_COUNT);
        result.add("Entry Type");
        result.add("Citation Key");


Please rename it to "CitationKey" as it is used in InternalField. Otherwise the column will not be removed.

priyanshu16095 · 2025-03-15T06:07:09Z

Please take a look at the documentation for codestyle.

https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-13-code-style.html

trag-bot · 2025-03-15T21:51:48Z

@trag-bot didn't find any issues in the code! ✅✨

priyanshu16095 · 2025-03-15T23:23:12Z

Tried it, works fine!

subhramit · 2025-03-16T00:42:09Z

+    public Set<String> getColumnNames() {
+        Set<String> result = new LinkedHashSet<>(columnCount + EXTRA_COLUMNS_COUNT);


Modern Java: https://www.baeldung.com/java-21-sequenced-collections -

subhramit · 2025-03-16T00:43:37Z

@priyanshu16095 Thank you for reviewing!

…s table. (JabRef#12699) * fix-consistency-check * Add import statement * Remove 'CitationKey' column from table * Use set * Update Checkstyle * Update checkstyle check * rename variable * Correct checkstyle * Correct checkstyle * Fix fail fast principle

fix-consistency-check

d9e731b

Paras-Gupta16 changed the title ~~fix-consistency-check~~ Remove second "Citation key" column from the Consistency check results table. Mar 12, 2025

github-actions Bot reviewed Mar 12, 2025

View reviewed changes

Add import statement

41cd3c7

koppor reviewed Mar 12, 2025

View reviewed changes

Paras-Gupta16 added 2 commits March 14, 2025 23:33

Remove 'CitationKey' column from table

76fcbf4

Use set

b18bc1a

priyanshu16095 suggested changes Mar 14, 2025

View reviewed changes

Update Checkstyle

a0bec0d

priyanshu16095 suggested changes Mar 14, 2025

View reviewed changes

Paras-Gupta16 added 2 commits March 15, 2025 11:17

Update checkstyle check

32e8e65

rename variable

5b4d5dd

Paras-Gupta16 added 3 commits March 16, 2025 03:07

Correct checkstyle

4c7b7c6

Correct checkstyle

6576b38

Fix fail fast principle

0e63378

subhramit reviewed Mar 16, 2025

View reviewed changes

subhramit approved these changes Mar 16, 2025

View reviewed changes

subhramit added this pull request to the merge queue Mar 16, 2025

Merged via the queue into JabRef:main with commit f8f5a38 Mar 16, 2025

		public Set<String> getColumnNames() {
		Set<String> result = new LinkedHashSet<>(columnCount + EXTRA_COLUMNS_COUNT);

Uh oh!

Conversation

Paras-Gupta16 commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor commented Mar 12, 2025

Uh oh!

priyanshu16095 commented Mar 12, 2025

Uh oh!

priyanshu16095 commented Mar 12, 2025

Uh oh!

koppor commented Mar 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priyanshu16095 Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priyanshu16095 commented Mar 15, 2025

Uh oh!

trag-bot Bot commented Mar 15, 2025

Uh oh!

priyanshu16095 commented Mar 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit commented Mar 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Paras-Gupta16 commented Mar 12, 2025 •

edited

Loading

priyanshu16095 Mar 14, 2025 •

edited

Loading