emacs-helm / helm-org

53 stars 9 forks source link

Use dynamic completion #10

Closed thierryvolpiatto closed 4 years ago

thierryvolpiatto commented 4 years ago

DON'T MERGE, just a proof of concept, let me know people with large org files if it's faster. Thanks. @alphapapa ping.

helm-org-startup-visibility filter have been disabled for now as it is not working for some reasons (seems also it slowdown helm-org).

When submitting a pull request, please include the following information:

alphapapa commented 4 years ago

Hi Thierry,

Sorry, I don't understand what this new helm-dynamic-completion and match-dynamic thing is. I looked at some commits to the main Helm repo that add them, but I don't understand what it's doing or what its purpose is. Would you explain, please?

If this speeds up the headings-in-buffer sources, that would be great, as long as the heading visibility feature doesn't regress in functionality.

Thanks.

thierryvolpiatto commented 4 years ago

alphapapa notifications@github.com writes:

Hi Thierry,

Sorry, I don't understand what this new helm-dynamic-completion and match-dynamic thing is. I looked at some commits to the main Helm repo that add them, but I don't understand what it's doing or what its purpose is. Would you explain, please?

Actual helm-org is taking a list of org filenames (or buffers) as the :candidates, and compute the list of all headings in those files with :candidates-in-buffer. Using :match-dynamic with helm-dynamic-completion allow computing directly the headings in :candidates with all-completions which is fast and provide as well in addition of usual multi matching what user set in completion-styles, it can be flex or helm-flex (similar to fuzzy) depending on the emacs version in use.

If this speeds up the headings-in-buffer sources, that would be great, as long as the heading visibility feature doesn't regress in functionality.

It's what I want to know, but I have not huge org buffers so it doesn't make a significant difference, thus the function used with helm-dynamic-completion is may be not adapted perfectly for this use. The only functionality that I have removed is the filtered-candidate-transformer function helm-org-startup-visibility.

Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

What can be done to speed up this in addition of dynamic completion is doing the same as what we are doing with helm-imenu. AFAIU we are using find-file-noselect when fetching agenda files and parsing the resulting buffer for fetching headings, and the these buffers are staying open. So when doing the initial call we could set 2 local vars in these buffers, one that handle the buffer-tick and one that handle a table containing all headers in this buffer. For next calls of helm-org we could use the data saved in each buffer unless buffer-tick have changed, in this case rebuild the heading list in this buffer only. Of course the first call will be the same than what we have actually but subsequent calls would be very fast even if something changed in one of the buffers. I will perhaps implement this in next days...

thierryvolpiatto commented 4 years ago

I have pushed the code with cache for individual buffers, please try, I would like to have feedback from people with huge org files. Thanks.

matthuszagh commented 4 years ago

Thanks for taking a look at this @thierryvolpiatto! The solution works very well with my largest org file ~1.1MB. The lag used to be several seconds. I no longer notice any lag (after the cache is generated).

alphapapa commented 4 years ago

Using :match-dynamic with helm-dynamic-completion allow computing directly the headings in :candidates with all-completions which is fast and provide as well in addition of usual multi matching what user set in completion-styles, it can be flex or helm-flex (similar to fuzzy) depending on the emacs version in use.

Thanks, but I don't understand what this new helm-dynamic-completion thing is or how it works. :) Why is it faster?

Regarding the caching of headings: That's a nice idea, but it seems like a lot of complexity to add to this package. Rather than implementing that in helm-org, you might consider looking at org-ql. I've already implemented a per-buffer cache, which caches the return value of functions at buffer positions, in the function org-ql--value-at: https://github.com/alphapapa/org-ql/blob/a1851ed0eb6587e822b62d18a884c1eabb1411c1/org-ql.el#L444 If you have a function which returns all headings in a buffer, you could call org-ql--value-at at point-min (in widened buffer) for your function, and it would cache the return value until the buffer tick changes. It could also be used to cache child or parent headings at a certain position. For example, see how it's called here: https://github.com/alphapapa/org-ql/blob/a1851ed0eb6587e822b62d18a884c1eabb1411c1/org-ql.el#L427

You might also consider using org-ql as a backend for listing headings. For example, this query returns all headings in a file, and it quickly retrieves them from the cache when run again:

(org-ql-select "~/org/main.org"
  '(regexp "*")
  :action #'org-ql--outline-path)

For retrieving headings matching certain regexps, a search with the terms can be used, like:

(org-ql-select "~/org/main.org"
  '(heading "word1" "word2")
  :action #'org-ql--outline-path)

The helm-org-ql command also makes such queries easy and fast: https://github.com/alphapapa/org-ql/blob/master/helm-org-ql.el For example, you can perform that same search with helm-org-ql by typing h:word1,word2. It would be easy to make a new command that searched only headings by keyword, regexp, etc, to avoid having to type the h: and commas.

Some ideas. :)

thierryvolpiatto commented 4 years ago

alphapapa notifications@github.com writes:

Using :match-dynamic with helm-dynamic-completion allow computing directly the headings in :candidates with all-completions which is fast and provide as well in
addition of usual multi matching what user set in completion-styles, it can be flex or helm-flex (similar to fuzzy) depending on the emacs version in use.

Thanks, but I don't understand what this new helm-dynamic-completion thing is or how it works. :)

See https://github.com/emacs-helm/helm/issues/2165

Why is it faster?

Initialization of candidates is not faster, but once done matching is faster because it is done with all-completions which run in C. The benefit also is that you have flex (aka fuzzy) matching for free if needed by using completion-styles. The actual implementation of helm-org is anyway wrong, it abuse candidates-transformer and prevent adding any other functionality.

Regarding the caching of headings: That's a nice idea, but it seems like a lot of complexity to add to this package.

It is not, I wrote it in ten minutes.

Rather than implementing that in helm-org, you might consider looking at org-ql.

Maybe, but I don't want to enter into the complexity of another package, and the purpose here is not my usage but how to improve helm-org to e.g. fix issue #1964 linked here.

Thanks.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

matthuszagh notifications@github.com writes:

Thanks for taking a look at this @thierryvolpiatto! The solution works very well with my largest org file ~1.1MB.

Great, thanks for testing.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

About org-hide-leading-stars and org-startup-indented. For the first one there is nothing to do (no need of a filtered-candidate-transformer like what was done before) as long as jit-lock ran in org buffer. For the second, what is the purpose of indenting the helm buffer? I propose to remove its support. I already removed the filtered-candidate-transformer fn which IMO is usefulness and slowdown even more things.

alphapapa commented 4 years ago

For the second, what is the purpose of indenting the helm buffer? I propose to remove its support.

When outline paths are not displayed, subheadings are displayed indented to their level. Without that indentation, outline structure would appear flattened, only distinguishable by faces, which are not necessarily distinctive, depending on user configuration. Removing support for that feature would be a significant regression in functionality.

alphapapa commented 4 years ago

@thierryvolpiatto The new cache seems to return the wrong results when relevant settings have changed. For example:

  1. Call helm-org-in-buffer-headings.
  2. C-g.
  3. (setq helm-org-format-outline-path t).
  4. Call helm-org-in-buffer-headings again. The results from step 1 are displayed instead of the outline path.
  5. Insert a character into the Org buffer.
  6. Call helm-org-in-buffer-headings again. The outline paths are now displayed.
thierryvolpiatto commented 4 years ago

alphapapa notifications@github.com writes:

@thierryvolpiatto The new cache seems to return the wrong results when relevant settings have changed. For example:

  1. Call helm-org-in-buffer-headings.
  2. C-g.
  3. (setq helm-org-format-outline-path t).
  4. Call helm-org-in-buffer-headings again. The results from #1 are displayed instead of the outline path.
  5. Insert a character into the Org buffer.
  6. Call helm-org-in-buffer-headings again. The outline paths are now displayed.

Works now if you use customize in 3. Note that you can as well refresh with C-u if needed.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

alphapapa notifications@github.com writes:

For the second, what is the purpose of indenting the helm buffer? I propose to remove its support.

When outline paths are not displayed, subheadings are displayed indented to their level. Without that indentation, outline structure would appear flattened, only distinguishable by faces, which are not necessarily distinctive, depending on user configuration. Removing support for that feature would be a significant regression in functionality.

Ok, just wanted to know, will reenable this with a FCT ASAP.

Thanks.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

Eugene Yaremenko notifications@github.com writes:

Would you have a suggestion what function can be used instead? 🤔

Seems Spacemacs called this on a single filename? Maybe try to use helm-org--get-candidates-in-file?

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

JAremko commented 4 years ago

@thierryvolpiatto Will try, thanks. Too bad that helm-org--get-candidates-in-file looks like a private function: helm-org -- get-candidates-in-file

thierryvolpiatto commented 4 years ago

Eugene Yaremenko notifications@github.com writes:

@thierryvolpiatto Will try, thanks. Too bad that helm-org--get-candidates-in-file looks like a private function: helm-org -- get-candidates-in-file

I am not sure what you are doing with this function, but if you use it for a helm source, you can build directly your source with

(helm-org-build-sources (list your_file_or_buffer))

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997