org-tree-slide icon indicating copy to clipboard operation
org-tree-slide copied to clipboard

skip everything under a tree

Open lg2011 opened this issue 3 years ago • 10 comments

I encounter a problem with trees (headings) with the COMMENT string which are not ignored when running org-tree-slide-mode in org-tree-slide-presentation-profile. A tree with a heading such as (* COMMENT My heading) is not ignored. I checked the variable org-comment-string which is "COMMENT" and org-tree-slide-skip-comments which is t. I tried with org-tree-slide-20220112.142 (melpa) and org-tree-slide- 2.8.18 (nongnu) but could not have it work.

lg2011 avatar Mar 30 '22 13:03 lg2011

Could you please provide me the org.el version? And it is really appreciated if you could also provide me an example org file that you are going to use it as a presentation.

takaxp avatar Mar 30 '22 13:03 takaxp

Thanks for you fast answer; my Org mode version is 9.5.2. You will find attached a small example file. example.org.txt [I added a txt extension since org files can not be uploaded directly)

lg2011 avatar Mar 30 '22 14:03 lg2011

I think I understand what is happening: the COMMENTed heading is indeed ignored but not the sub-headings inside which make the COMMENTed heading visible as a parent of these sub-headings (the sub-headings of a COMMENTed heading are not considered as COMMENTed as well). I thought that it was a kind of "skip" the tree and all its subtrees but this was not the correct interpretation. May be it would be useful to have such a "skip everything under a tree" functionality.

lg2011 avatar Mar 30 '22 14:03 lg2011

Thank you for providing the version information and an example. Your understanding is correct since current implementation will show a tree without COMMENT under COMMENTed parent tree.

takaxp avatar Mar 30 '22 14:03 takaxp

The following replacement of the function org-tree-slide--heading-skip-comment-p seems to provide expected (at least for me) behaviour:

(defun org-tree-slide--heading-skip-comment-p ()
  "Return t, if the current heading or parent heading is commented."
  (and org-tree-slide-skip-comments
       (and (eval (cons 'or (mapcar (lambda (x)  (string-match (concat "^" org-comment-string) x)) (org-get-outline-path t))))) t))

Since I'm not (at all) an elisp programmer, this could probably be largely improved.

lg2011 avatar Mar 31 '22 17:03 lg2011

I'm trying to update the current implementation under develop branch. https://github.com/takaxp/org-tree-slide/commit/4214474dfa17073c19beb00c94f81be5e18350fb

Thank you for proposing a code, it is great for me so I just integrate it to new version but the feature should be provided as an option for user so org-tree-slide-skip-comments has been extended to support the new feature. When user set inherit to org-tree-slide-skip-comments, every heading under COMMENTed parent heading will be skipped as you proposed.

org-tree-slide-skip-comments-toggle is also updated to support the inherit skipping feature without breaking previous workflow. I'll provide some hooks to existing profiles so that user can change org-tree-slide-skip-comments even when applying a profile.

I'll test the new code little bit more. Thank you for pulling the trigger of my actions :)

takaxp avatar Apr 02 '22 04:04 takaxp

Thank you for the improved solution, I checked the source code and it reminded me of the lisp instruction memq :wink: . I will try the development version on my side and will make a feedback in the next days.

lg2011 avatar Apr 02 '22 09:04 lg2011

I made a quick test of the updated source code and here are my findings:

  1. in line 525, should not the instruction be (message "COMMENT: HIDE (inherit)") instead of (message "COMMENT: SHOW (inherit)")
  2. I think there is the problem with the update of the org-tree-slide-skip-comments when org-tree-slide-comments-toggle is executed.

When org-tree-slide-skip-comments is in state inherit then org-tree-slide-comments-toggle changes it to nil which is correct.

When org-tree-slide-skip-comments is in state nil then org-tree-slide-comments-toggle changes it to inherit which is correct.

However when org-tree-slide-skip-comments is in state nil (after being in state inherit) and when you start then stop a presentation then org-tree-slide-comments-toggle changes it to t which is (I think) not expected since at this stage the toggle is only between t and nil, it is not possible to switch back to the initial state inherit. I could not understand (sorry) why there was this change of inherit <-> nil cycle after launching presentation in a nil state.

Otherwise the inherit state is working as expected, thank you very much for the changes.

lg2011 avatar Apr 03 '22 12:04 lg2011

Thank you for your feedback. All suggestions will be reflected to the next release. The toggle feature should be changed to cycle states like (nil, t, inherit) instead of toggle but it depends on use cases so I'll think of that little bit more.

takaxp avatar Apr 03 '22 14:04 takaxp

I modified the toggle feature in order to stick to the following workflow: inherit (skip all tree) -> t (skip only current level) -> nil (show all) -> inherit. Also, there was a permutation between the messages HIDE and SHOW. Here is my code, I tested it in various contexts, and it seems to work well:

(defun org-tree-slide-skip-comments-toggle ()
"Toggle show COMMENT item or not.
Toggle according to the following cycle:  inherit (skip all COMMENTed tree) -> t (skip only current COMMENTed level)  -> nil (show all) -> inherit
"

  (interactive)
 
  ;; Toggle
  (cond ((eq org-tree-slide-skip-comments 'inherit) (setq org-tree-slide-skip-comments t))
	((eq org-tree-slide-skip-comments t) (setq org-tree-slide-skip-comments nil))
	((eq org-tree-slide-skip-comments nil) (setq org-tree-slide-skip-comments 'inherit)))
  
  ;; Feedback
  (cond ((eq org-tree-slide-skip-comments nil)
         (message "COMMENT: SHOW"))
        ((eq org-tree-slide-skip-comments t)
         (message "COMMENT: HIDE"))
        ((eq org-tree-slide-skip-comments 'inherit)
         (message "COMMENT: HIDE (inherit)")))
  (setq org-tree-slide--slide-number
        (format " %s" (org-tree-slide--count-slide (point)))))

lg2011 avatar Apr 04 '22 15:04 lg2011

Hi, I have just started using org-tree-slide and I love it (way better than org-present) and I encounter this issue as well. I had checked the develop branch and it seems good to be merged. I had applied the code for my emacs and it matched my expected behaviour when used in org-mode

If we have to nitpick I guess these are possible changes:

  1. Make org-tree-slide-skip-comments-toggle only callable in org-mode and fix its feedback and doc string.
(defun org-tree-slide-skip-comments-toggle ()
  "Toggle show COMMENT item or not.
Toggle `org-tree-slide-skip-comments' between `t' and `nil'.
If `org-tree-slide-skip-comments' is `inherit',
then toggle between `inherit' and `nil'."
  (interactive)
  (if (eq major-mode 'org-mode)
      (progn
        ;; Sync
        (if (eq org-tree-slide-skip-comments 'inherit)
            (setq org-tree-slide--skip-comments-mode 'inherit)
          (when (eq org-tree-slide-skip-comments t)
            (setq org-tree-slide--skip-comments-mode t)))
        ;; Toggle
        (if (eq org-tree-slide--skip-comments-mode 'inherit)
            (if (eq org-tree-slide-skip-comments 'inherit)
                (setq org-tree-slide-skip-comments nil)
              (setq org-tree-slide-skip-comments 'inherit))
          (setq org-tree-slide-skip-comments
                (not org-tree-slide-skip-comments)))
        ;; Feedback
        (cond
         ((eq org-tree-slide-skip-comments nil)
          (message "COMMENT: SHOW"))
         ((eq org-tree-slide-skip-comments t)
          (message "COMMENT: HIDE"))
         ((eq org-tree-slide-skip-comments 'inherit)
          (message "COMMENT: HIDE (inherit)")))
        (setq org-tree-slide--slide-number
              (format " %s" (org-tree-slide--count-slide (point)))))
    ;; if not in org mode, org-complex-heading-regexp is nil which will fail
    ;; org-get-outline-path that is called by org-tree-slide--parent-commented-p
    ;; when doing org-tree-slide--count-slide.
    (message "Cannot toggle outside of org-mode.")))

This is because this interactive function calls org-tree-slide--count-slide, which will check for skipped headings. Ultimately, org-tree-slide--parent-commented-p -> org-get-outline-path -> org--get-outline-path-1 will be called. org--get-outline-path-1 expects org-complex-heading-regexp to be not nil, but it is nil when not in org-mode. This means calling it outside of org-mode will produce error. 2. improve custom of org-tree-slide-skip-comments to sth like this:

(defcustom org-tree-slide-skip-comments t
  "Specify to skip COMMENT item or not.
t: skip only headings with COMMENT, child headings without COMMENT will be shown.
nil: show headings even if it has COMMENT
inherit: skip headings with COMMENT and its child headings.
"
  :type
  '(choice
    (const :tag "Skip only headings with COMMENT" t)
    (const :tag "Show headings even if it has COMMENT" nil)
    (const
     :tag
     "Skip headings with COMMENT and its child headings"
     inherit))
  :group 'org-tree-slide)
  1. Doc string for org-tree-slide--get-parents should use headings instead of headlines for consistency. This: "Get parent ~headlines~ headings and concat them with DELIM."
  2. replace obsolete functions org-show-entry org-show-siblings and point-at-bol, FYI, the *Compile-Log*(deduplicated):
‘org-show-entry’ is an obsolete function (as of 9.6); use ‘org-fold-show-entry’ instead.
‘org-show-siblings’ is an obsolete function    (as of 9.6); use ‘org-fold-show-siblings’ instead.
‘point-at-bol’ is an obsolete function (as of 29.1); use ‘line-beginning-position’ or ‘pos-bol’ instead.

For point-at-bol I recommend line-beginning-position since from Emacs 29.1 point-at-bol is just alias for line-beginning-position.

ed9w2in6 avatar Aug 23 '23 05:08 ed9w2in6

@takaxp I am using my stated changes and it seems good so far, not rigorously tested though. I can submit a pull request if you prefer that.

ed9w2in6 avatar Aug 23 '23 05:08 ed9w2in6

@ed9w2in6 Thank you for the feedback and your practical comments. We can check if the org-mode is active or not by org-tree-slide--active-p, so I'd like to use it. Thus, I'll integrate the changes on develop branch and your suggestions to the main branch as soon as possible with your name in a commit message.

Additionally, regarding issues on the obsolete functions are handled in https://github.com/takaxp/org-tree-slide/issues/61. I'd like to resolve these as separate commit. Could you please put your comment in the issue page?

takaxp avatar Aug 23 '23 05:08 takaxp

EDIT: never mind! I'll just check out the development branch once this is applied! TY!

mplscorwin avatar Aug 23 '23 05:08 mplscorwin