Skip to content

Order StandardActions like the File menu - #777

Open
koppor wants to merge 1 commit into
mainfrom
standardactions-file-menu-order
Open

koppor wants to merge 1 commit into
mainfrom
standardactions-file-menu-order

Conversation

@koppor

@koppor koppor commented Sep 23, 2026

Copy link
Copy Markdown
Member

Summary

The constants for the Git menu entries sat at the very end of StandardActions, far from the other entries for sharing a library, so their position said nothing about where they appear in the File menu. They now sit next to the shared-database entries, in the order the File menu uses. Nothing changes for users.

jabref-contrib-policy:4.2:reviewed​:ok

Analogies

Like a bar of chocolate, this menu comes in labelled squares, and this pull request puts the squares back into the rows they belong to. The honey is the same honey, only the jars now stand in the order one reaches for them. And like the moon, the menu keeps showing the same face it always did.

Steps to test

  1. Open a library.
  2. Open the File menu: the entries appear unchanged, with Git and Shared database next to each other above Preferences.

File menu

Related issues and pull requests

Triggered by JabRef#14493

AI usage

Claude Code (model claude-opus-5), AIL4.

AI CHECKLIST.md walkthrough

Code checklist

[!IMPORTANT]
This is a mandatory final gate. When the implementation is finished and before you open a PR, work through every box below and tick it. If a box cannot be ticked, fix the code first; mark a box [/] only if the point genuinely does not apply.

AGENTS.md describes how to write code while developing — this file confirms the finished result. Do not skip the checklist and do not skip individual points.

1. Code self-review

Read your own diff once, top to bottom, and confirm each point.

Nullability and control flow

  • [/] No == null / != null checks — JSpecify annotations (@NullMarked, @Nullable, @NonNull) used instead.
  • [/] No Objects.requireNonNull(...) — nullability expressed via JSpecify annotations.
  • [/] New classes annotated with @NullMarked (org.jspecify.annotations.NullMarked).
  • [/] New packages annotated with @NullMarked (org.jspecify.annotations.NullMarked).
  • [/] Optional consumed with ifPresent / ifPresentOrElse / map / orElseThrow — never orElse(unusedValue) nor an isPresent() + get() block.
  • [/] StringUtil.isBlank(...) used instead of s == null || s.isBlank().

Exceptions

  • [/] No catch (Exception e) — only specific exceptions are caught.
  • [/] No throw new RuntimeException(...) / IllegalStateException(...) — these tear down the whole application.
  • [/] Logged exceptions are passed as the last logger argument (LOGGER.info("...", e)), not concatenated into the message string.

Style and idioms

  • [/] New BibEntry objects built with withers (withField, not setField).
  • [/] Modern Java used: List.of() / Map.of() / Set.of(), Path.of(), SequencedCollection / SequencedSet, text blocks.
  • [/] Regexes use a precompiled Pattern.compile(...) constant, not String.matches(...).
  • [/] Keep alphabetical ordering when adding variables or enum items.
  • [/] Background work uses org.jabref.logic.util.BackgroundTask, not new Thread().
  • No commented-out code, no trivial comments restating the code, no AI-disclosure comments in source.
  • [/] Markdown Javadoc (///) uses Markdown syntax, not JavaDoc inline tags: `code` instead of {@code}, [ClassName] instead of {@link}.

User-facing text

  • [/] All user-facing text localized (Localization.lang in Java, % prefix in FXML).
  • [/] Sentence case (not Title Case); no trailing !; labels do not end with :.
  • [/] Variance expressed with placeholders ("...: %0"), not string concatenation.

Security

  • [/] User-controlled data (request params, entry fields, file contents) is HTML-escaped before being written into any text/html response — including exception/error messages, not just the success body (XSS).

Tests

  • [/] Behavior changes in org.jabref.model / org.jabref.logic have added or updated tests.
  • [/] Tests assert object contents (assertEquals), use plain JUnit asserts (not AssertJ), have no @DisplayName, do not catch exceptions (let them propagate so JUnit reports setup/teardown failures directly), and use @TempDir instead of manual temp directories.
  • [/] Fetcher tests hit the live endpoints — the remote API is not mocked or stubbed (automated-review suggestions to mock it are rejected on purpose).

2. Verification commands

Run in this order — cheapest first. Each must pass.

  • [/] ./gradlew :jablib:check (or ./gradlew check for all modules).
  • ./gradlew checkstyleMain checkstyleTest checkstyleJmh.
  • ./gradlew modernizer.
  • ./gradlew --no-configuration-cache :rewriteDryRun reports no changes (run ./gradlew rewriteRun to fix).
  • ./gradlew javadoc.
  • [/] npx markdownlint-cli2 "docs/**/*.md" "*.md" (only if Markdown changed).
  • [/] npm ci && npm run textlint reports no misspellings (only if Markdown changed).
  • [/] Only if formatting is still off after rewriteRun: docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml".

3. Documentation

  • [/] CHANGELOG.md entry added if the change is visible to the user (end-user wording, no extra blank lines, sorted in next to existing entries about the same component/feature). Link the issue if one exists; link the PR only when no issue exists. Use TODO as the placeholder when neither is known yet — never a fake number. No entry for fixes to changes that were themselves introduced after the last release (feature only in ## [Unreleased]) — update the existing unreleased entry instead if needed.
  • [/] Searched jabref/issues and jabref-koppor/issues for a related issue; linked only on a confident match, otherwise kept TODO (no closes/fixes for merely-similar issues).
  • [/] Requirement added to docs/requirements/<area>.md if the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes).
  • [/] Developer documentation under docs/ updated if behavior or architecture changed.

4. Pull request

  • PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
  • All checklist items kept and marked [x], [ ], or [/].
  • "Steps to test" is a numbered list with a cropped screenshot of the result for every visible change; no video (only allowed when another program is involved, e.g. drag and drop or push to an external application).
  • All HTML comments removed from the PR body.
  • PR created with gh pr create --body-file <file> (not --body).
  • [/] If CHANGELOG.md used a TODO placeholder (no issue confidently identified yet — an existing issue link always stays), the PR was opened as draft, the placeholder was replaced with the real PR-number link after PR creation, committed and pushed, and only then was the PR marked ready for review. If an issue is identified or created later, the link is switched to the issue.

Not applicable throughout: the change moves enum constants, it adds no code, no user-facing text and no behavior.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • [/] I added one sentence (max 20 words) to CHANGELOG.md describing the change from the user's point of view (if the change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

Not applicable: no behavior change, so nothing to test, document or note in the changelog.

🤖 Generated with Claude Code

https://claude.ai/code/session_018RuRpcKSiQEzrbZ1eWX7bx

The Git entries sat far from the other library-sharing entries, so their position in the file said nothing about where they appear in the menu.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018RuRpcKSiQEzrbZ1eWX7bx
@koppor koppor added this to the 1 - soon-upstream milestone Sep 23, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Align StandardActions ordering with the File menu

✨ Enhancement 🕐 Less than 5 minutes

Grey Divider

AI Description

• Moves Git actions beside shared-database actions in StandardActions.
• Aligns declaration order with File menu grouping without changing behavior.
High-Level Assessment

The chosen approach is appropriate because declaration order now mirrors the existing File menu grouping. Adding comments or separate metadata would introduce maintenance overhead without improving this straightforward organizational refactor.

Files changed (1) +11 / -8

Refactor (1) +11 / -8
StandardActions.javaGroup Git actions with File menu sharing actions +11/-8

Group Git actions with File menu sharing actions

• Relocates the Git action constants from the end of the enum to the library-sharing section immediately before shared-database actions. Action labels, icons, and runtime behavior remain unchanged.

jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant