Skip to content

Fix repository folder drops on the Dock icon - #765

Merged
hannesa2 merged 1 commit into
gitx:masterfrom
johndouthat:fix-dock-folder-drop
Oct 11, 2026
Merged

Fix repository folder drops on the Dock icon#765
hannesa2 merged 1 commit into
gitx:masterfrom
johndouthat:fix-dock-folder-drop

Conversation

@johndouthat

@johndouthat johndouthat commented Oct 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dragging a repository folder onto GitX's Dock icon is rejected on macOS 26.6.2 because the app declares folders using the legacy fold OSType. Declare public.folder with LSItemContentTypes and associate it with PBGitRepositoryDocument, so macOS accepts the drop and AppKit opens the repository through the existing document flow.

Fixes #305.

Validation:

  • Built current master (008ad6cc) and confirmed Launch Services reported acceptsFolder=NO.
  • Regression tests fail before the fix for ordinary and dotted folder names; the fixed build accepts both and preserves .git folder support.
  • xcodebuild test -only-testing:GitXTests: 324 tests passed.
  • Document-type tests verify both Launch Services acceptance and AppKit document routing.
  • xcodebuild test -only-testing:GitXUITests: 8 UI tests passed, including delivery of a repository-folder open event through Launch Services.
  • The focused folder-open UI test also verifies the reopened history table and captures a screenshot.
  • Reviewed by GPT-6 Astra with high reasoning; no remaining findings.

Manually verified that dragging a repository folder from Finder onto GitX’s Dock icon opens the repository successfully. Automated tests also verify macOS’s folder-drop acceptance and repository opening through Launch Services.

@johndouthat
johndouthat marked this pull request as ready for review October 10, 2026 12:17
@hannesa2
hannesa2 requested a balanced review from Copilot October 10, 2026 21:18
Copilot AI reviewed Oct 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The document declaration is correctly updated and covered by focused unit and UI regression tests.

0 open findings

What changed in this PR

Updates GitX’s document declaration so macOS accepts repository folders dropped onto the Dock icon, resolving #305.

Changes:

  • Declares folder support using public.folder and routes it to PBGitRepositoryDocument.
  • Adds unit and UI regression coverage for Launch Services folder opening.
File Description
Resources/​Info.plist Modernizes the folder document-type declaration.
GitXUITests/​GitXScreenshotTests.m Tests repository opening through Launch Services.
GitXTests/​PBGitXDocumentTypesTests.m Verifies folder acceptance and document routing.
GitX.xcodeproj/​project.pbxproj Adds the document-type tests to the test target.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@goneng goneng added the bugfix label Oct 10, 2026
@hannesa2
hannesa2 requested a review from goneng October 11, 2026 05:06
hannesa2
hannesa2 approved these changes Oct 11, 2026
View reviewed changes
Comment thread GitXTests/PBGitXDocumentTypesTests.m
@@ -0,0 +1,45 @@
#import <XCTest/XCTest.h>

@hannesa2 hannesa2 Oct 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, it even includes tests.

@hannesa2
hannesa2 merged commit 336e35f into gitx:master Oct 11, 2026
3 checks passed
@goneng

goneng commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks for tracking this down, John. The public.folder + NSDocumentClass change looks right to me, and the two unit tests in PBGitXDocumentTypesTests pass in CI.

I ran make ui-test locally on 9a5be58 (Xcode, arm64) and have two suggestions about the new tests.

  1. testRepositoryFolderOpensThroughLaunchServices fails for me at the first wait (GitXScreenshotTests.m:146). After Cmd-W the window is still there, even after waiting 40 seconds. When I made that wait non-throwing, the rest of the test passed, but the original window was never closed. In that case the later checks ("window reappears", "history table exists") are met by the window that was already open, and openURLs:withApplicationAtURL: may only activate it. So as written the test may not show that the Launch Services open event reopens the repository. I don't know why Cmd-W is a no-op on my machine, so it may well work on yours. A more robust version could close the document another way, wait until the window is gone, and then check that a new window appears after the open event.

  2. git clang-format --diff reports changes on the lines this PR adds in both test files (@[ a, b ] spacing, wrapped URLByAppendingPathComponent: calls, argument alignment, and the { of the new test method). make format should fix it.

@hannesa2

hannesa2 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Oh sorry, it looks like that I merged too early

@goneng

goneng commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

These are minor fixes - can be submitted as a followup if we want.

hannesa2 added a commit that referenced this pull request Oct 11, 2026
@hannesa2
Address review feedback on PR #765 tests
6fb23ca 
- testRepositoryFolderOpensThroughLaunchServices: close the repository
  window via its close button instead of Cmd-W, which was not reliably
  closing the window on every machine. The expectation now asserts the
  wait actually completed, and a comment clarifies that any window seen
  after the Launch Services open event must be newly created, since the
  original was confirmed fully closed beforehand.
- Run `make format` to apply clang-format to the lines added in
  PBGitXDocumentTypesTests.m and GitXScreenshotTests.m (array literal
  spacing, wrapped URLByAppendingPathComponent: calls, argument
  alignment, and brace placement on the new test method).

Co-authored-by: Copilot <[email protected]>
@hannesa2

hannesa2 commented Oct 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

These are the follow up changes #767

hannesa2 added a commit that referenced this pull request Oct 12, 2026
@hannesa2
Merge pull request #767 from gitx/fixTest
5baf289 
Address review feedback on PR #765 tests
goneng pushed a commit to goneng/gitx that referenced this pull request Oct 22, 2026
@hannesa2
Address review feedback on PR gitx#765 tests
25735db 
- testRepositoryFolderOpensThroughLaunchServices: close the repository
  window via its close button instead of Cmd-W, which was not reliably
  closing the window on every machine. The expectation now asserts the
  wait actually completed, and a comment clarifies that any window seen
  after the Launch Services open event must be newly created, since the
  original was confirmed fully closed beforehand.
- Run `make format` to apply clang-format to the lines added in
  PBGitXDocumentTypesTests.m and GitXScreenshotTests.m (array literal
  spacing, wrapped URLByAppendingPathComponent: calls, argument
  alignment, and brace placement on the new test method).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@hannesa2 hannesa2 hannesa2 approved these changes
@goneng goneng Awaiting requested review from goneng

Assignees

No one assigned

Labels

bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature request: Accept folder drags

4 participants

@johndouthat @goneng @hannesa2