Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Looking for feature suggestions #3

Closed
integral-dw opened this issue Feb 12, 2020 · 27 comments
Closed

Looking for feature suggestions #3

integral-dw opened this issue Feb 12, 2020 · 27 comments
Labels
enhancement New feature or request

Comments

@integral-dw
Copy link
Owner

So, as I previously hinted, I have taken a quick look at the open issues and suggestions over at org-bullets. Here's what I found:

  • Perfomance Questions 🖥️ 🔥
    It seems that there were performance issues related to the rendering of UTF8 characters. This seems to have been primarily an issue on windows, but sadly I currently have no way to test that. I would appreciate checking this package's performance on a modern windows computer!
  • TODO-specific header bullets ☑️
    Another interesting suggestion was to add special bullets for TODO and DONE headings, as suggested by @sumitsahrawat. I would like to know if people are generally interested in such a feature.
  • Bug Reports 🦋
    Different codebase, different bugs. Currently I have squashed all bugs I could find, but I have more faith in my ability to produce bugs than in my ability to find them. If any strange behaviour crops up, be sure to open an issue!

That seems to cover the issue tracker of org-bullets. I would greatly appreciate comments regarding the points I found, how they may be applicable to or desirable for this package.

Thanks to everyone in advance!

@integral-dw integral-dw added enhancement New feature or request help wanted Extra attention is needed labels Feb 12, 2020
@alphapapa
Copy link

  • TODO-specific header bullets

On one hand, that seems out-of-scope for this package that is supposed to be like a reimplementation of org-bullets. But on the other hand, it does seem like a similar idea, and if you could provide a convenient UI to flexibly do this for to-do keywords (and list checkboxes, maybe?), so that users wouldn't have to set up compose-region or `pretty-symbols or something like that manually, that would seem helpful. I don't know if I would end up using it in my own config, but I'd probably try it out and see if I like it.

@integral-dw
Copy link
Owner Author

On one hand, that seems out-of-scope for this package that is supposed to be like a reimplementation of org-bullets.

Yeah, I have minor reservations because of that, but it's hard to strike a balance between avoiding feature creep and giving the project a unique enough identity to not have it feel redundant.

But on the other hand, it does seem like a similar idea, and if you could provide a convenient UI to flexibly do this for to-do keywords (and list checkboxes, maybe?), so that users wouldn't have to set up compose-region or `pretty-symbols or something like that manually, that would seem helpful.

TODO keywords should not be a problem in general, although I would restrict the scope of this package to essentially "non-interactive" components. Checkboxes (which a user may want to interact with, see org-mouse) seem to demand their own package.

Funnily enough, I do have plans for creating a package converting a particularly ingenious prettification for SRC blocks from a context-insensitive prettify-symbols-mode extension to a fully context-sensitive, interactive font-lock extension, ideally with mouse support. I think prettify-symbols is great for prose, but I personally find it a bit difficult for code because it cannot change it's behavior depending on a symbol's context. Depending on how complex that code gets, I would likely tackle the issue of prettifying things like checkboxes there.

Back on the topic of a UI for prettifying TODO keywords, I am thinking of a nice and clean plain alist on the user end. I would probably put a decent amount of tweaking into the custom interface and focus on getting that nice and intuitive, with the alist itself representing the data more plainly.

@alphapapa
Copy link

Thanks for that link! I hadn't seen his blog before. Another great example of a blog generated with Org. I added it to https://alphapapa.github.io/org-almanac/#Blogging.

Your plans sound good. Thanks.

@lmintmate
Copy link

lmintmate commented Feb 16, 2020

I've been watching this package since I found out it exists, and would like to replace my own fork of org-bullets with it when it's ready for prime-time. A feature I would thus love to see (and which I implemented in my fork of org-bullets) is an option to remove the indentation from the bullets, so that each is underneath the other, instead of the tabbed hierarchy (useful if the characters used for the stars make obvious the level of the heading (e.g. letters or numbers), and indentation is thus superfluous), found from this PR to the original org-bullets repository. In my fork I basically implemented the changes of this PR by hand, as the original forked repo was deleted and I also had forked the emacsorphanage version, which had made some incompatible changes in the meantime. Thanks in advance for your consideration.

P.S. In general, you could get some more ideas for features from the other PRs from the original org-bullets repository too.

@lmintmate
Copy link

Also, in regards to the performance issues, I have encountered the aforementioned lag issue for windows pcs in org-superstar as well (Windows 10, emacs 26.3), and I solved it in the same way too, that is setting inhibit-compacting-font-caches to t.

@integral-dw
Copy link
Owner Author

An excellent point, I forgot the PRs! I am currently working on the TODO feature (almost done I believe). Once that is out of the way I'll take a look at how to implement this feature nicely. I think I will implement it the way org implements hiding emphasis markers, though.

@integral-dw
Copy link
Owner Author

Regarding the performance issue, could you do me a favor and try out whether a plain text file full of bullets (C-x 8 RET FISHEYE RET) causes stuttering with inhibit-compacting-font-caches set to nil?

@integral-dw
Copy link
Owner Author

Looking at the third pull request, the primary change seems to be adding an explicit mouse event for visibility cycling. I would leave that out for the time being, in order to keep elements provided by the package largely non-interactive.

@lmintmate
Copy link

lmintmate commented Feb 17, 2020

Regarding the performance issue, could you do me a favor and try out whether a plain text file full of bullets (C-x 8 RET FISHEYE RET) causes stuttering with inhibit-compacting-font-caches set to nil?

Weirdly enough, I don't see any stuttering in a plain text (*.txt) file full of fisheye characters with inhibit-compacting-font-caches set to nil. But as soon as I go to an org file (e.g. the README of my emacs config, which is fairly big - maybe that plays a role too) with org-superstar mode enabled and inhibit-compacting disabled, the stuttering starts after attempting to scroll and/or add another phrase in the text of said file. Maybe then the issue isn't inherently with the UTF-8 characters, but with something in org-mode and/or org-bullets/org-superstar.

@integral-dw
Copy link
Owner Author

Ah, I think I know the cause of the issue. Here's a MWE function of what the modes do to compose the bullets:

(defun mwe (start end)
  "May cause tremendous slowdown when used in region."
  (interactive "r")
  (let ((pos start))
    (while (< pos end)
      (if (save-excursion (goto-char pos) (eolp))
          (setq pos (1+ pos))
          (compose-region pos (setq pos (1+ pos))
                          ?◉)))))

This will convert every (non-newline) character to a fisheye (visually). Try opening a larger buffer, mark a region, and call mwe. It should cause the same slowdown.

@integral-dw integral-dw removed the help wanted Extra attention is needed label Feb 17, 2020
@lmintmate
Copy link

lmintmate commented Feb 18, 2020

This will convert every (non-newline) character to a fisheye (visually). Try opening a larger buffer, mark a region, and call mwe. It should cause the same slowdown.

Just took a .txt file with about 60 lorem ipsum paragraphs and ran mwe on about 30 of them, while the inhibit compacting option was set to nil. I see hardly any slowdown; if there is, it's certainly not to the degree of org-bullets/org-superstar. Maybe something with org-mode is to blame then.
P.S. I tried to create an org buffer with lorem ipsums and run mwe on it, but it doesn't work there. How could this code be modified as to be able to work on an org-mode buffer (in order to test if that mode is then to blame, or even if the modification required to make this work in org-mode causes the slowdown)?

@integral-dw
Copy link
Owner Author

Ah yes, it's a font-lock matter then, I believe. Here's a minor mode using font lock:

(defun slo-down (start end)
  "May cause tremendous slowdown when used in region."
  (let ((pos start))
    (while (< pos end)
      (if (save-excursion (goto-char pos) (eolp))
          (setq pos (1+ pos))
          (compose-region pos (setq pos (1+ pos))
                          ?◉)))))

(defvar slo-keys
  '(("^[A-Ma-m].*$"
        (0 (slo-down (match-beginning 0)
                     (match-end 0))))))

(define-minor-mode slo-mode
  "Try to slow down the buffer."
  nil " Slo" nil
  (cond
   (slo-mode
    (font-lock-add-keywords nil slo-keys 'append)
    (font-lock-ensure)
    (font-lock-flush))
   (t
    (decompose-region (point-min) (point-max))
    (font-lock-remove-keywords nil slo-keys)
    (font-lock-ensure)
    (font-lock-flush))))

This will convert any line beginning with a letter between a and m, you can easily play with the regex however to see how far you can go (worst case being ^.*$ of course)

@integral-dw
Copy link
Owner Author

If that does not break it it's likely not the bullets but the different faces. Then I'd probably whip up a function applying like a dozen different faces to each line.

@lmintmate
Copy link

Again I did not manage to trigger a slowdown like the one that happens in org-bullets/org-superstar with inhibit-compacting set to nil (even in org-mode, with org-superstar disabled of course so as not to interfere in the results). On the other hand, if the font-lock was the culprit, I would have experienced a similar slowdown as a result of packages that add extra syntax highlighting via font-lock (e.g. https://github.com/cireu/elispfl, my fork of which with added features I have enabled in my own config), which hasn't been the case.
P.S. Btw, I just now noticed that you added the option to remove the indentation (via org-superstar-remove-leading-stars), and set it to t in my experimental org-superstar config. It works really well, so thanks for adding it! I'm definitely switching to org-superstar as soon as it hits MELPA.

@integral-dw
Copy link
Owner Author

It works really well, so thanks for adding it! I'm definitely switching to org-superstar as soon as it hits MELPA.

Glad to hear you like it! Regarding the slowdown issue, I'll have to try cutting superstar-mode down to size to try and extract another potential MWE. Try this: load superstar and org as you normally would, but eval this afterwards in the scratch buffer:

(defun org-superstar--update-font-lock-keywords ()
  "Set ‘org-superstar--font-lock-keywords’ to reflect current settings.
You should not call this function to avoid confusing this mode’s
cleanup routines."
  (setq org-superstar--font-lock-keywords
        `(("^\\(?3:\\**?\\)\\(?2:\\*?\\)\\(?1:\\*\\) "
           (1 (org-superstar--prettify-main-hbullet) prepend)))))

Now the package only prettifies UTF-8 bullets and merges two fonts. If this triggers it then I can boil it down further into two variants, one of which will be the MWE. Thank you so much for your patience and continued help! 😄

@alphapapa
Copy link

@integral-dw It's great to see the care with which you are developing this package. Thank you.

@lmintmate
Copy link

Regarding the slowdown issue, I'll have to try cutting superstar-mode down to size to try and extract another potential MWE. Try this: load superstar and org as you normally would, but eval this afterwards in the scratch buffer:

(defun org-superstar--update-font-lock-keywords ()
  "Set ‘org-superstar--font-lock-keywords’ to reflect current settings.
You should not call this function to avoid confusing this mode’s
cleanup routines."
  (setq org-superstar--font-lock-keywords
        `(("^\\(?3:\\**?\\)\\(?2:\\*?\\)\\(?1:\\*\\) "
           (1 (org-superstar--prettify-main-hbullet) prepend)))))

Now the package only prettifies UTF-8 bullets and merges two fonts. If this triggers it then I can boil it down further into two variants, one of which will be the MWE.

Trying to test this, I had a different problem now: When I at first got and tried to use the latest version of the org-superstar.el file, upon attempting to load an org file I got this error:

File mode specification error: (void-variable org-hide-leading-stars-before-indent-mode)

and org-superstar wasn't enabled.

This error is obviously the result of commit 6fcf4b5 and has to do with the fact that org-hide-leading-stars-before-indent-mode is sort of a hidden variable. I don't recall what I saw when I did the testing fot the melpa thread yesterday unfortunately, but I believe it didn't appear in the regulardescribe-variable (C-h v) list until I did the setq-local. At any rate, I thankfully had around the version of the file prior to this commit (I use a quasi-manual bootstrapping system leveraging url-copy-file for packages not on MELPA, which means that the files only get updated when I delete the existing version from the lisp folder. Thankfully the old version was still in the Recycle Bin when I saw the problem with the new version, so I was able to restore it (and even if it were deleted, I probably could have used the github interface to browse the repository prior to the offending commit and thus get the old version that way). If org-hide-leading-stars-before-indent-mode is indeed a variable that should be ignored (as you state in the melpa thread), this commit should probably be reverted, as it prevents org-superstar from even being enabled.

After I got past this problem, replacing the problematic new version with the old one, after eval'ing the above code snippet and restarting org-superstar, I now got the lag (again having inhibit-compacting set to nil). Just to verify something: this changed the fontification so that all stars would show, but only the last one was fontified as a bullet - this was the expected visual result, right?

@alphapapa
Copy link

@lmintmate You may find some of these tools useful for installing and upgrading packages not on MELPA: https://github.com/alphapapa/unpackaged.el#packages

@integral-dw
Copy link
Owner Author

Just to verify something: this changed the fontification so that all stars would show, but only the last one was fontified as a bullet - this was the expected visual result, right?

Correct! I'll sit down later and cobble together a more compact working example.

Very strange that an error cropped up there, but it should be fixed now on both branches.

@integral-dw
Copy link
Owner Author

Ah, my patch has been accepted. org-hide-leading-stars-before-indent-mode is now deprecated! 😄

integral-dw added a commit that referenced this issue Feb 22, 2020
This file serves as a working example for the Issue discussed here:
#3
@integral-dw
Copy link
Owner Author

Okay, I added a new version of slo-mode to the test directory. This one should behave the same as messing with superstar's innards (at least for test-case = 1). Could you try loading this file with the three different settings for test-case, @lmintmate?

@lmintmate
Copy link

Okay, I added a new version of slo-mode to the test directory. This one should behave the same as messing with superstar's innards (at least for test-case = 1). Could you try loading this file with the three different settings for test-case, @lmintmate?

Weirdly enough, I see lag for all 3 test cases. In particular, I tried scrolling down quickly, adding text and selecting (with the mouse) text in order to delete it. I see in all cases lag when scrolling down and when selecting text and deleting the selected text (though for the latter less often in case 2). I also saw lag very occasionally when adding text too. Unfolding headlines also lagged a little bit the first time I did it (though more so for cases 1 and 3).

Just to be clear, the visual result for test cases 1 and 3 is that the last star of the line becomes a fisheye instead, while for test case 2 there is no obvious visual change, is that right?

@integral-dw
Copy link
Owner Author

Just to be clear, the visual result for test cases 1 and 3 is that the last star of the line becomes a fisheye instead, while for test case 2 there is no obvious visual change, is that right?

Yep, I guess I know what's going on now. I boiled the thing down to what I think is more or less an MWE. Try test-case 1 and 2. There will be no visual differences whatsoever.

@lmintmate
Copy link

Yep, I guess I know what's going on now. I boiled the thing down to what I think is more or less an MWE. Try test-case 1 and 2. There will be no visual differences whatsoever.

Now, I still see lag on both test cases when scrolling quickly, but on test case 1 I don't see lag when adding text and selecting text and deleting the selected text, while on 2 I do occasionally. And indeed there is no obvious visual change on both cases. I also don't see lag when unfolding all headlines on both cases (unless one counts the slight lag when enabling slo-mode).

@integral-dw
Copy link
Owner Author

It's peculiar that you see any lag in test case 1, but that essentially means it's a deep-rooted font-lock issue. Case 2 essentially allows you to "stack" faces on top of one another, which I use to dynamically adapt the face of the bullet alongside the heading face. Quoting the documentation of inhibit-compacting-font-caches:

Some large fonts cause lots of consing and trigger [garbage collection]. If they
are removed from the font caches, they will need to be opened
again during redisplay, which slows down redisplay. If you
see font-related delays in displaying some special characters,
and cannot switch to a smaller font for those characters, set
this variable non-nil.

From the looks of it this issue stems not from special UTF-8 characters (or anything particular to the modes), but simply the (default) font used in conjunction with the particular system. It can likely be triggered by any fontlock-reliant mode. I'll put up a small explanation in the README about this and add the fix.

integral-dw added a commit that referenced this issue Feb 25, 2020
This also covers what I believe is all I can do regarding the slowdown
issue discussed here: #3
@integral-dw
Copy link
Owner Author

I think this update pretty much wraps this issue up. Thanks again for your help, @lmintmate!
Should I close this issue now and finally bump the version to 1.0.0?

@alphapapa
Copy link

Regarding this:

  • optional display of two leading bullets for inline tasks :: This
    feature is perfectly within the scope of this package.
    However, I will not add it unless asked, to avoid preemptive
    accumulation of features no one asked for.

I confess that I hardly use inline tasks at all, so I'm not requesting this. However, cleaning up the stars from inline tasks would be a very cool feature. So if you're looking for an excuse to do it, you can say that someone actually commented on it. ;) Maybe a nice idea for 1.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants