emacs-helm / helm-org

53 stars 9 forks source link

Update helm-org.el #15

Closed ahlearn closed 4 years ago

ahlearn commented 4 years ago

Add ":follow 1" to helm-org-build-sources.

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

Note that Emacs and Org versions persist "in the wild" for some time after release, so it is not appropriate to only test with the latest released or development versions of Org; tests must include versions still commonly in use. Proposed changes must not break functionality for existing users.

thierryvolpiatto commented 4 years ago

Thanks, but I am not sure everyone would like this change. I have nothing against using :follow in helm-org, I use it myself sometimes. But it is easy when you want to follow to use C-c C-f and if you use helm-follow-mode-persistent follow will be enabled persistently. Don't know what other org users think @alphapapa thoughts ?

alphapapa commented 4 years ago

I don't think this is a good idea. Jumping to results may cause, as a side effect, the outline folding to change.

And as you pointed out, Thierry, this is easily done with the key binding and customization options, so I don't see any reason to do this, either. AFAIU, follow-mode has never been intended to be a default behavior.

So this change would seem to be one user's preference being imposed on others as a default, which shouldn't be done.

So it's a NAK from me. If I may be so bold, as a sort-of co-maintainer of helm-org, I'm closing this now.

thierryvolpiatto commented 4 years ago

alphapapa notifications@github.com writes:

I don't think this is a good idea. Jumping to results may cause, as a side effect, the outline folding to change.

And as you pointed out, Thierry, this is easily done with the key binding and customization options, so I don't see any reason to do this, either. AFAIU, follow-mode has never been intended to be a default behavior.

So this change would seem to be one user's preference being imposed on others as a default, which shouldn't be done.

So it's a NAK from me. If I may be so bold, as a sort-of co-maintainer of helm-org, I'm closing this now.

Agree for all, thanks.

-- Thierry

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

ahlearn commented 4 years ago

Thanks for the detailed feedback. I did have (setq helm-follow-mode-persistent t) in my config and it works fine for most helm except helm-org-build-sources, and that's why I proposed this patch. Let me know if I missed somethings. Thanks.

alphapapa commented 4 years ago

I did have (setq helm-follow-mode-persistent t) in my config and it works fine for most helm except helm-org-build-sources, and that's why I proposed this patch. Let me know if I missed somethings.

The patch would force follow mode in all cases, so it is not the right way to fix the problem.

If there's a bug that prevents follow mode from working, please report that as a bug. If you want to propose a patch, please follow the code to the source of the problem.

thierryvolpiatto commented 4 years ago

ahlearn notifications@github.com writes:

Thanks for the detailed feedback. I did have (setq helm-follow-mode-persistent t) in my config and it works fine for most helm except helm-org-build-sources,

Can't reproduce, follow is working fine here with helm-org and it becomes persistent as soon as I hit C-c C-f and persistence is disabled when I hit again C-c C-f.

and that's why I proposed this patch. Let me know if I missed somethings. Thanks.

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

-- Thierry

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