Skip to content

refactor: consolidate fringe management functions#285

Draft
gggion wants to merge 1 commit intonobiot:mainfrom
gggion:refactor/fringe-management
Draft

refactor: consolidate fringe management functions#285
gggion wants to merge 1 commit intonobiot:mainfrom
gggion:refactor/fringe-management

Conversation

@gggion
Copy link
Copy Markdown
Contributor

@gggion gggion commented Dec 26, 2025

Fringe management functions consolidated into:

;;;;; Core Operations
(defun org-transclusion--make-fringe-string (face))
(defun org-transclusion--prefix-has-fringe-p (prefix))

;;;;; Region Operations
(defun org-transclusion-add-fringes (buffer beg end face))
(defun org-transclusion-remove-fringes (buffer beg end))

With this I’ve also adapted all places where the previous fringe placemente functions where being called in org-transclusion.el and org-transclusion-indent-mode.el

such as

org-transclusion-indent-mode.el

-(declare-function org-transclusion-prefix-has-fringe-p "org-transclusion" (prefix))
-(declare-function org-transclusion-add-fringe-to-region "org-transclusion" (buffer beg end face))
-(declare-function org-transclusion-remove-fringe-from-region "org-transclusion" (buffer beg end))
+(declare-function org-transclusion--prefix-has-fringe-p "org-transclusion" (prefix))
+(declare-function org-transclusion-add-fringes "org-transclusion" (buffer beg end face))
+(declare-function org-transclusion-remove-fringes "org-transclusion" (buffer beg end))
-                                 (not (org-transclusion-prefix-has-fringe-p line-prefix)))
-                        (org-transclusion-add-fringe-to-region
+                                 (not (org-transclusion--prefix-has-fringe-p line-prefix)))
+                        (org-transclusion-add-fringes
-                (org-transclusion-add-fringe-to-region
+                (org-transclusion-add-fringes
-      (org-transclusion-remove-fringe-from-region src-buf src-beg src-end))))
+      (org-transclusion-remove-fringes src-buf src-beg src-end))))

Same deal on org-transclusion.el

Besides refactoring the fringe functions, I’ve also added an hybrid approach that applies fringes through different mechanisms depending on org-indent-mode being enabled or not.

This was due to the following problem: If indent-mode and the transclusion-indent-mode extension are disabled, the per-line text property approach is unable to preserve fringes in the source when executing revert-buffer. The original implementation before the indent-mode additions did not have this issue because it placed the fringes as overlay properties, which survived buffer-revert and other changes to the buffer..

So, since fringe placement through overlays is only an issue if we want to preserve indentation, I’ve made it so fringes will be placed through overlays WHEN org-indent-mode is disabled. If not, then we’ll default to the per line text property approach.

[!NOTE] If the extension org-transclusion-indent-mode is not enabled but org-indent-mode is, indentation will be preserved, but fringes will still be lost to revert-buffer and other font locking operations on the transclusion source buffer, so the extension is still necessary if we want fringes to work properly with indent-mode.

Another detail is that when org-indent-mode is active without the transclusion-indent-mode extension loaded, fringes can still be reapplied during normal buffer modifications via the overlay modification hook, but NOT after revert-buffer (which doesn't trigger modification hooks when content is unchanged). Users who need full org-indent-mode support including buffer reverts should load the org-transclusion-indent-mode extension.

SUMMARY:

Retaining fringes - Scenarios Normal Editing revert-buffer, font-lock/etc Description
indent-mode OFF YES (overlay) YES (overlay) Works as intented, overlay ensures fringe is retained and adapts to changes in buffer
indent-mode ON, no extension YES (hook) NO Fringe will survive manual buffer modification, but will be lost on font-lock and other buffer operations such as rever-buffer
indent-mode ON, with extension YES (extension) YES (extension) Fringe will be reapplied in case of text-property erasure, works without issues.

* org-transclusion.el (org-transclusion-add-fringes): Renamed from
`org-transclusion-add-fringe-to-region'. Now uses overlay properties
when `org-indent-mode' is inactive (which survive `revert-buffer'),
and text properties when active (to preserve per-line indentation).
(org-transclusion-remove-fringes): Renamed from
`org-transclusion-remove-fringe-from-region'. Handles both overlay
and text property removal based on `org-indent-mode' state.
(org-transclusion--prefix-has-fringe-p): Renamed from
`org-transclusion-prefix-has-fringe-p' with double-dash to indicate
internal function.
(org-transclusion--make-fringe-string): New helper function to create
fringe indicator strings for both graphical and terminal displays.
(org-transclusion-source-overlay-modified): Simplified to only
reapply text properties when `org-indent-mode' is active but the
indent-mode extension is not loaded. Guard reference to
`org-transclusion-indent-mode' with `boundp' check to avoid
void-variable errors when extension is not loaded.

* org-transclusion-indent-mode.el: Update all function calls to use
new names: `org-transclusion-add-fringes',
`org-transclusion-remove-fringes', and
`org-transclusion--prefix-has-fringe-p'.

The original implementation before the indent-mode additions used
overlay properties for fringes, which survived buffer reverts.  The
initial per-line text property approach was necessary for
`org-indent-mode' compatibility but broke the non-indent case where
fringes disappeared after `revert-buffer' in the source buffer.  This
hybrid approach uses overlay properties when `org-indent-mode' is
inactive (preserving `revert-buffer' behavior) and text properties
when active (preserving per-line indentation from org-indent).

When `org-indent-mode' is active without the indent-mode extension
loaded, fringes are reapplied during normal buffer modifications via
the overlay modification hook, but NOT after `revert-buffer' (which
doesn't trigger modification hooks when content is unchanged). Users
who need full `org-indent-mode' support including buffer reverts
should load the `org-transclusion-indent-mode' extension.

Summary:
- indent-mode DISABLED: uses overlays for fringe placement, survives
  through revert-buffer.

- indent-mode ENABLED + transclusion-indent-mode DISABLED: uses
  text-properties for fringe placement, which dissapear after executing
  revert-buffer on the transclusion source (acceptable).

- indent-mode ENABLED + transclusion-indent-mode ENABLED: fringes are
  reapplied after revert-buffer while the transclusion is active.

Addresses nobiot#284
@gggion
Copy link
Copy Markdown
Contributor Author

gggion commented Dec 26, 2025

STATUS: currently debugging graphical and terminal emacs instances:

  • org-indent-mode: enabled/disabled

  • org-transclusion-indent-mode enabled/disabled

  • org-modern: this seems to be tricky, apparently there's a lot of font-locking business going on that make fringe and text properties behave differently when org-indent-mode is enabled or disabled, if there's no workaround it might require a new user option to set fringe placement mechanism (for example always use overlays regardless).

  • org-modern-indent-mode enabled/disabled.

@gggion
Copy link
Copy Markdown
Contributor Author

gggion commented Jan 2, 2026

@nobiot one question: would it be better to make this PR directly to the feat/transient branch?
I've made it for main but since it'll be merged to feat/transient and have to undergo other changes for compatibility, I'm thinking of making this for feature/transient in the first place since that feature branch willl be merged with main eventually anyways.

@nobiot
Copy link
Copy Markdown
Owner

nobiot commented Jan 3, 2026

@gggion ,

one question: would it be better to make this PR directly to the feat/transient branch?

I have just merged the feat/transient branch with main branch. It is significantly diverged from the main branch before the merge. Could I ask you to look into rebasing before proceeding any further?

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.

2 participants