Skip to content

file-manager: classify git worktrees and submodules with distinct icon labels - #1893

Open
Inspirati wants to merge 4 commits into
mate-desktop:masterfrom
Inspirati:feature/git-context-labels
Open

file-manager: classify git worktrees and submodules with distinct icon labels#1893
Inspirati wants to merge 4 commits into
mate-desktop:masterfrom
Inspirati:feature/git-context-labels

Conversation

@Inspirati

Copy link
Copy Markdown
Contributor

Summary

Extends the git branch display feature in Caja's icon view to visually
distinguish three types of git-tracked directories:

Directory type Label
Standard repo [branch]
Linked worktree [branch ⎇]
Submodule [branch ⊂]

The ⎇ symbol (U+2387, ALTERNATIVE KEY) is widely used by git prompt tools
to denote linked worktrees. The ⊂ symbol (U+2282, SUBSET OF) reflects that
a submodule is a repository contained within another.

The suffix is appended for both normal branch names and detached HEAD
labels (e.g. [detached: abc1234 ⎇]).

Implementation

Classification is performed inside get_git_branch() after the gitdir
path has been fully resolved, by checking for the well-known path
components that git always uses:

if (strstr (effective_gitdir, ".git/worktrees/") != NULL)
    gitdir_type = GITDIR_WORKTREE;
else if (strstr (effective_gitdir, ".git/modules/") != NULL)
    gitdir_type = GITDIR_SUBMODULE;
else
    gitdir_type = GITDIR_STANDARD;

A file-scoped GitdirType enum encodes the three cases. No changes are
required outside fm-icon-container.c.

Relationship to PR #1892

This branch includes the bug fix from PR #1892 (double-free and
realpath() for relative gitdir: paths) as a prerequisite commit,
since the feature builds directly on the effective_gitdir variable
introduced by that fix. Maintainers may choose to merge #1892 first and
rebase this branch, or merge both together.

Testing

A representative test tree covering all three cases can be created with:

# Standard repository
git init main-repo && cd main-repo
git checkout -b main
echo "test" > README.md && git add . && git commit -m "init"

# Linked worktree (produces .git file with absolute gitdir path)
git checkout -b feature-xyz && git checkout main
git worktree add ../feature-worktree feature-xyz

# Submodule (produces .git file with relative ../  gitdir path)
cd .. && git init container-repo && cd container-repo
git checkout -b main
echo "container" > README.md && git add . && git commit -m "init"
git -c protocol.file.allow=always submodule add ../main-repo submodule
git commit -m "Add submodule"

Navigate to the parent of main-repo/ and feature-worktree/ in Caja
icon view to see [main] and [feature-xyz ⎇] respectively. Navigate
into container-repo/ to see [main ⊂] on the submodule/ icon.

The submodule case (gitdir: ../.git/modules/submodule) also exercises
the relative-path crash fix from PR #1892 — if enumeration of
container-repo/ completes without crashing, both fixes are working.

Inspirati and others added 2 commits June 3, 2026 18:43
… pointers

When a .git file contains a relative gitdir: path (as used by submodules
and linked worktrees), get_git_branch() was calling g_free() on its own
const char* parameter, then the caller would free the same pointer again.
This double-free causes heap corruption that manifests as a crash when
enumerating a parent directory containing such a submodule.

Fix by introducing a separate resolved_gitdir variable to hold the
heap-allocated resolved path, leaving the parameter untouched. Use
realpath() to canonicalise ../ components in relative paths, and guard
against a NULL return when the target path does not exist on disk.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n labels

Extend get_git_branch() to inspect the resolved gitdir path and classify
each .git entry as a standard repository, a linked worktree, or a
submodule. Append a Unicode suffix to the branch label for non-standard
cases so users can distinguish directory types at a glance in icon view:

  Standard repo  →  [branch]
  Linked worktree →  [branch ⎇]
  Submodule      →  [branch ⊂]

Classification is performed by searching the resolved gitdir path for the
well-known path components .git/worktrees/ and .git/modules/, which git
always uses for these object types. A file-scoped GitdirType enum encodes
the three cases.

The suffix is appended to both normal branch names and detached HEAD
labels. No changes are required outside fm-icon-container.c.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@vkareh vkareh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Caveat: I do not like this feature being part of core caja. It should live in caja-extensions. That said, the code is good (with some change requests).

A few additional requests:

  • Remove the Anthropic ad from the commit messages (or at least change to Assisted-by: Claude Sonnet 4.6 to avoid linking the Claude account to MATE commits)
  • Reword the commit messages for human readability (it currently is too verbose with too many internal code details, rather than a concise description of the problem and how it solves it - look at the few latest commits in https://github.com/mate-desktop/caja/commits/master for examples)

Comment thread src/file-manager/fm-icon-container.c Outdated
Comment on lines +367 to +368
const char *suffix = (gitdir_type == GITDIR_WORKTREE) ? " ⎇" :
(gitdir_type == GITDIR_SUBMODULE) ? " ⊂" : "";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would these render in all systems, or would it show a tofu box instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not know, and it is beyond my scope to find out. I went with it because it looks good compared to the alternative options considered. If there is downstream feedback about it being a problem, it can be revisited later. In possible cases of them not rendering, at least it should fail safe with the tofu box.

Run-time determination of the available character set is beyond the scope of the feature introductory effort.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, but I expect more due diligence than "out of scope" when it's actually the purpose of the feature you're submitting.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Until the possibility of rendering issues being reported, the current implementation should fail safe. If such is reported further investigations can be undertaken.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Until the possibility of rendering issues being reported, the current implementation should fail safe. If such is reported further investigations can be undertaken.

You're proposing a "fix after merge" solution to a "test before merge" problem.

No.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we do "fix after merge" or even "test after merge" with code AI played any role in, people will lose trust in MATE. Bad enough to do that with our own code but it we do that with AI we break faith with millions who do not trust AI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have already tested it and it works perfectly. What I am proposing is that if an issue is identified, then it shall be investigated - just like with any other software artefact.

Clearly.

I was going by what you said earlier:

I do not know, and it is beyond my scope to find out.

If you've tested this in a variety of standard fonts and they all work consistently, then that's the "due diligence" part I'm referring to.

My original review question was:

Would these render in all systems, or would it show a tofu box instead?

Seems like you have a reasonable answer to that question then. You've tested across a variety of setups and did not encounter tofu boxes, yes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens if an intentionally bad font is thrown at it? If I were an attacker that's one of the things I would try. "Tofu boxes" if this is what I think it is would be OK, a segfault would not be.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct. I'm just disclosing that upon looking at the list of distributions on the Caja GitHub page https://github.com/mate-desktop/caja - I have not tried on many of these. For example, I'm not about to try configuring a build system under Cygwin in order to test for these rendering as tofo boxes or not. Another option may be to have the strings mapped into the translation system, but then I would think this is not really an issue of language translation, but rather of font support on all possible systems. When I implemented the original feature, I ended up spending way more time on the string translations than I did on the underlying code. And even then, I wasn't able to cover all the currently supported languages.

@lukefromdc lukefromdc Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To test on all distros is not what I'm asking and is certainly beyond my own capacity with no landline and no VM experience. Rather I would favor testing with a variety known good and then some known bad fonts to see what happens. That should cover most cases.

Same basic idea as testing a passphrase handling gui with a good passphrase, then a bad one, and finally with a whole page of text cut and pasted in while running from terminal. The last looks for buffer overflows in that case.

Comment thread src/file-manager/fm-icon-container.c Outdated
Comment thread src/file-manager/fm-icon-container.c Outdated
Comment thread src/file-manager/fm-icon-container.c Outdated
@lukefromdc

lukefromdc commented Jun 9, 2026 via email

Copy link
Copy Markdown
Member

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.

4 participants