Kungsgeten / org-brain

Org-mode wiki + concept-mapping
MIT License
1.72k stars 102 forks source link

Fix org-link issue in entry section #387

Open hwiorn opened 1 year ago

hwiorn commented 1 year ago

Org 9.6 makes links in org-fold broken. So it needs to add overlay style.

This issue already happened in other org-mode based packages. e.g: org-roam

Fix #382

vantu5z commented 1 year ago

Apply this fix and it does not work for me. Entry section still show full links. Until I add (setq org-fold-core-style 'overlays) to config, it was text-properties by default.

hwiorn commented 1 year ago

@vantu5z How did you apply this patch? Do you use native-comp? What is your emacs version?

Strangely, current org-brain doesn't work correctly with bytecompile(#352) which doom uses default. Check org-brain.elc in ~/.emacs.d/.local/straight/build-*/org-brain/ If there is the file, this patch doesn't be applied well. For bytecompiling and polymode, org-brain needs this patch(#386).

If you want this fix to work in your doom, follow this trick.

vantu5z commented 1 year ago

What is your emacs version?

system Arch Linux Linux 6.1.12-arch1-1 x8664 x emacs 28.2 ~/.emacs.d/ doom 3.0.0-pre PROFILE=@0 HEAD -> master 5c563d307 2023-02-25 20:47:29 -0500

How did you apply this patch?

For bytecompiling and polymode, org-brain needs this patch(https://github.com/Kungsgeten/org-brain/pull/386).

Try to apply both patches 386 and 387, but this does not help.

Remove patches and set variable org-fold-core-style to overlays in config, and after that links show as expected. Docs say:

org-fold-core-style is a variable defined in org-fold-core.el.

Internal implementation detail used to hide folded text.

Can be either text-properties or overlays. The former is faster on large files, while the latter is generally less error-prone with regard to third-party packages that haven't yet adapted to the new folding implementation.

Important: This variable must be set before loading Org.

hwiorn commented 1 year ago

@vantu5z

Run doom build -r to rebuild.

Can you show your ~/.emacs.d/.local/straight/build-*/org-brain/ directory? Check org-brain.elc and remove it if there is. I had tested Emacs 28.2, and I had the same phenomenon as you when the org-brain.elc file was existed. After the file was removed, this fix works.

My version is

GNU Emacs     v28.2            73da6d4ad5750e73767401740f282be272831c3e
Doom core     v3.0.0-pre       HEAD -> master 5c563d307 2023-02-25 20:47:29 -0500
Doom modules  v23.02.0-pre     HEAD -> master 5c563d307 2023-02-25 20:47:29 -0500

For byte-compile issue, #386 and #387 works on Emacs 30.0.5 already on my another machine without the issue. I test 28.2 for backward compatible based on stable doomemacs only

On org-roam and doomemacs, some users got fixed by org-roam#2236 patch, Some users are not. They use (setq org-fold-core-style 'overlays) directly like you to fixup before org-mode loading. I think org-fold-core-style issue seems to occur by user's config differently.

Refer these: doomemacs#6542 and org-roam#2198

But, As you know, setting org-fold-core-style globally has a downside. The performance issue and org-cycle issue(org-roam#2198 comment). That's why I set the variable locally in this patch.

vantu5z commented 1 year ago

Can you show your ~/.emacs.d/.local/straight/build-*/org-brain/ directory?

$ ls ~/.emacs.d/.local/straight/build-28.2/org-brain/
org-brain-autoloads.el  org-brain.el  org-brain.elc

After the file was removed, this fix works.

Yes, after remove elc file, this fix works.

hwiorn commented 1 year ago

Yes, after remove elc file, this fix works.

Ok, that's good news. I think that indicate the #386 patch didn't be applied well also.

How did you apply this patch?

* Go to `~/.emacs.d/.local/straight/repos/org-brain/` and fetch github data by magit, merge this PR (commits from this PR applied to `org-brain.el`).

* Run `doom build -r` to rebuild.

* Restart emacs.

For bytecompiling and polymode, org-brain needs this patch(#386).

Try to apply both patches 386 and 387, but this does not help.

If you update the package.el or repos directory, you need to remove build directory of org-brain. And doom build -r doesn't rebuild org-brain. You have to run doom sync, actually. Refer my previous comment.

TIP: On Emacs 28.2, If you got ("org-brain" (error "org-brain.el:0:0: error: cl-assertion-failed: ((> (point) output-start))")) error constantly, remove ~/.emacs.d/.local/straight/build-* directory itself and run doom sync -p.

hwiorn commented 1 year ago

I found that Emacs 28.x and byte-compile of doomemacs does optimize the org-fold-core-style let-binding. So I change let to setq-local. Emacs 30.0.50 doesn't have this byte-compile issue.

@vantu5z Can you check this PR once again without removing org-brain.elc?

vantu5z commented 1 year ago

@hwiorn

Can you check this PR once again without removing org-brain.elc?

Yes, now it works with compilation.

vantu5z commented 1 year ago

It should be separate issue, but ask here as it seems to be related to org layout: Entry section shows part of org content but formatting is not accurate (indents, bullets, maybe some other things). It is more visible when use showchildren special tag:

hwiorn commented 1 year ago

It should be separate issue

Yeah, I think it needs to be a separate issue and patch. I think that is enhancement, not a bug.

Entry section shows part of org content but formatting is not accurate (indents, bullets, maybe some other things). It is more visible when use showchildren special tag:

* headings and text, lists below have no indents;

* `***` and list `+` is not replaced by unicode symbols (`org-superstar`).

Simply, I can tell, It needs org-indent-mode and org-superstar-mode under Entry section.

I'm not a maintainer and don't have the permission of this. The best thing is @Kungsgeten has to decide that enhancement request.

hwiorn commented 1 year ago

@Kungsgeten Could you check this PR?

hwiorn commented 1 year ago

@vantu5z Could you post the enhancement feature as a new issue?

I think other users could discuss it.

vantu5z commented 1 year ago

@hwiorn Add new issue #388