Keep colors and add Org mode support

[larstvei]

This pull request mainly addresses #2. It calculates a suitable foreground color for the region out of focus.

Doing this is significantly more resource demanding, so some effort has been made to avoid redundant computations. This includes to only recalculate when the focused area changes, only change the colors within the bounds of the window and ignore all invisible text (which can typically occur in Org-mode buffers with hidden subtrees).

The changes require a user to explicitly set a foreground in the focus-unfocused face when using Focus without a theme (#21 ). The reason for this is to allow this feature to co-exist with other customizations of the focus-unfocused and focus-focused faces (e.g. changing the size of the unfocused text, or making the focused area bold).

Additionally, using Focus with Org-mode has been improved, by using org-element to provide a reasonable chunk to Focus on.

[ReneFroger]

I'll will try this branch now. Thanks for sharing it!

[larstvei]

Any problems so far @ReneFroger? I’m leaning towards merging and releasing soon.

[alphapapa]

Doing this is significantly more resource demanding

You weren't kidding! :) I'm sorry to report that this is probably not usable in its current state. I loaded my main personal Org file into a clean Emacs, ran focus-mode, and began doing C-n across the first few lines, which are Org #+ headers and an Org list of one-line quotes. The delay in moving the point to the next line is around one full second.

However, when I copied only those lines into a separate Org file, without the several large subtrees after them, performance seemed fine. Maybe the org-element-based code needs to be optimized to avoid looking at the rest of the buffer.

[alphapapa]

BTW, I understand basically why you do this, but I think it's inappropriate to do so when the file is loaded, as that's out of the package's scope:

(put 'org-element 'bounds-of-thing-at-point
     #'focus-org-element-bounds)

[alphapapa]

Also, it seems bogus to be running this on every movement command:

(defun focus-get-thing ()
  "Return the current thing, based on `focus-mode-to-thing'."
  (or focus-current-thing
      (let* ((modes (mapcar 'car focus-mode-to-thing))
             (mode  (or (cl-find major-mode modes)
                        (apply #'derived-mode-p modes))))
        (if mode (cdr (assoc mode focus-mode-to-thing)) 'sentence))))

Calling mapcar on every movement command means consing and creating garbage just for moving the cursor around. Instead there should probably be a minor mode and/or a buffer-local variable that is set to the proper bounds-getting function.

[alphapapa]

BTW, I'm noticing some--what seems to me like--strange behavior with focus-mode enabled (and this in emacs -q with only Focus loaded):

• Edebug isn't usable to step through code. e.g. I instrumented focus-dim-area, but as soon as I press n or f or anything while Edebug is active, the Edebug session is aborted. I'm guessing this is due to post-command-hook being modified, but I don't understand why that would cause it, because focus-init adds to that hook buffer-locally.
• The profiler doesn't seem to be working properly. e.g. see this report:

• focus-move-focus 237 88%
  • focus-dim-area 236 88%
  • focus-make-unfocused-face 4 1% face-attr-construct 4 1%
  • focus-bounds 1 0%
  • bounds-of-thing-at-point 1 0%
  • focus-org-element-bounds 1 0%
  • org-element-at-point 1 0%
    • org-element--parse-to 1 0%
    • org-element--current-element 1 0% org-element--list-struct 1 0%
• ... 20 7% Automatic GC 20 7%
• command-execute 10 3%
  • call-interactively 10 3%
  • funcall-interactively 10 3%
  • profiler-start-stop-report 10 3%

    
    It shows that `focus-dim-area` is using the majority of CPU time, but when expanded, that node only shows `focus-make-unfocused-face`, which is only using 1%, leaving 87% unaccounted for.  I don't know what's going on here.

For convenience, I'm using this function for profiling:

(cl-defun profiler-start-stop-report (&optional (mode 'cpu+mem))
  (interactive (list (if current-prefix-arg
             'cpu
               'cpu+mem)))
  (if (profiler-running-p)
      (progn
    (profiler-report)
    (profiler-stop))
    (profiler-start mode)))

(define-key global-map (kbd "<f1>") #'profiler-start-stop-report)

So, e.g.:

1. Open Org buffer.
2. M-x focus-mode RET.
3. F1
4. C-n.
5. F1.

[larstvei]

Thank you @alphapapa! I'm glad I held off on merging. A full second is not an acceptable amount of lag. In the most recent version (9af4fbe) the lag was not noticeable on the Org-files I tested.

I loaded my main personal Org file into a clean Emacs, ran focus-mode, and began doing C-n across the first few lines, which are Org #+ headers and an Org list of one-line quotes. The delay in moving the point to the next line is around one full second.

Org-mode is problematic because it has a lot of invisible text displayed within the bounds of the window. Focus does ignore all invisible text, but it seems that the function that identifies the invisible regions (next-single-char-property-change) is slow. I don't think there is any hope in implementing a faster version in elisp, as it is a C function. This might be a show-stopper. BTW, it is almost certainly not related to org-element or getting the bounds (I'm not claiming this can't be optimized, but it's not what causes the dramatic lag you've reported)

BTW, I understand basically why you do this, but I think it's inappropriate to do so when the file is loaded, as that's out of the package's scope:

Here I used the same approach as lsp-mode. It does seem to be the intended way of customizing thingatpt. From the docstring:

Possibilities include symbol, list, sexp, defun, filename, url, email, word, sentence, whitespace, line, and page.

See the file thingatpt.el for documentation on how to define a valid THING.

Profiling has provided useful and reasonable results when I've used it, and I have not seen results like the ones you report. I'll try to reproduce it. As for Edebug, my guess is that the function does nothing when you try to step through because it is wrapped in a while-no-input-clause.

Thank you again for the very useful and detailed feedback! It might be that the approach simply doesn't scale. In that case, I'll either have to

• find a different approach entirely;
• identify the problematic cases (which I think is simply a huge amount of text within the bounds of the window), and revert to gray-style focus-mode in those cases;
• give up.

I hope to find a scalable way of doing this, using roughly the same approach. Either way, I'll be sure not to merge anything before things can run smoothly with more or less any emacs buffer (perhaps with the exception of files that are very slow with emacs by default (e.g. minified js files)).

[alphapapa]

Thank you @alphapapa! I'm glad I held off on merging. A full second is not an acceptable amount of lag. In the most recent version (9af4fbe) the lag was not noticeable on the Org-files I tested.

Yeah, the Org file I tested it on is 576 KB and 12k lines. I have larger ones, though. :)

Org-mode is problematic because it has a lot of invisible text displayed within the bounds of the window. Focus does ignore all invisible text, but it seems that the function that identifies the invisible regions (next-single-char-property-change) is slow. I don't think there is any hope in implementing a faster version in elisp, as it is a C function. This might be a show-stopper. BTW, it is almost certainly not related to org-element or getting the bounds (I'm not claiming this can't be optimized, but it's not what causes the dramatic lag you've reported)

This might be a silly idea, but what if you removed the checks for invisible text? Is that really necessary? In this case, there were large subtrees that were fully collapsed, i.e. only the top-level headings were visible, which means a lot of invisible text. Maybe checking for invisible text (at least, in Org buffers) is more work than necessary. Like I said, maybe a silly idea; I haven't tested it, and you know this code better than I do.

BTW, I understand basically why you do this, but I think it's inappropriate to do so when the file is loaded, as that's out of the package's scope:

Here I used the same approach as lsp-mode. It does seem to be the intended way of customizing thingatpt. From the docstring:

Possibilities include symbol, list, sexp, defun, filename, url, email, word, sentence, whitespace, line, and page. See the file thingatpt.el for documentation on how to define a valid THING.

Hmm, okay. You might consider proposing a patch upstream to Org or Emacs then, because that seems like it should already be a thing, and then you wouldn't need to do it here.

Profiling has provided useful and reasonable results when I've used it, and I have not seen results like the ones you report. I'll try to reproduce it.

Yeah, that was really weird. It was in a clean Emacs config (using makem.sh's sandbox feature), so it's really strange.

As for Edebug, my guess is that the function does nothing when you try to step through because it is wrapped in a while-no-input-clause.

Aha. Yeah, this is one of the areas that gets tricky in Emacs.

Thank you again for the very useful and detailed feedback! It might be that the approach simply doesn't scale. In that case, I'll either have to

• find a different approach entirely;
• identify the problematic cases (which I think is simply a huge amount of text within the bounds of the window), and revert to gray-style focus-mode in those cases;
• give up.

I hope to find a scalable way of doing this, using roughly the same approach. Either way, I'll be sure not to merge anything before things can run smoothly with more or less any emacs buffer (perhaps with the exception of files that are very slow with emacs by default (e.g. minified js files)).

Have you considered using font-lock for this? It already has functionality to limit changes to visible and changed parts of the buffer, and you could use a matching function that either applies face properties or works with an overlay.

Here's another idea that might work in conjunction with font-lock or with the code as-is: use narrowing where appropriate to prevent the code from looking into parts of the buffer that aren't relevant. Not sure if that could help, but it might in some cases.

Let me know if I can help more. I haven't used this package yet, but I'm keen to try it on my Org files if it becomes usable.