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

Better integration with dired-details invisible properties #139

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

nnicandro
Copy link
Contributor

For the filter groups, the approach is to take advantage of
the add-to-invisibility-spec and
remove-from-invisibility-spec functions when toggling the
filter groups. The semantics of buffer-invisibility-spec
allow the invisible property of some text to be a list of
symbols and if one of those symbols is a symbol in
buffer-invisibility-spec, the text is invisible.

This allows us to, instead of overwriting the invisible
property, upgrade it to a list of symbols thereby
preserving any invisibility symbols already present. This
is what the new function
dired-utils-fillin-invisible-property does. Our own
invisibility symbol is filled in when
dired-filter-group--apply is called and the visibility of
the group can be toggled by adding and removing the symbol
from the invisibility spec.

For dired-narrow, the new function
dired-utils-remove-invisible-property is used which
removes a specific invisibility symbol while preserving any
other ones. When the narrowed state is updated, one of
dired-utils-fillin-invisible-property or
dired-utils-remove-invisible-property is called depending
on if the current line passes the filter predicate. Since
no text properties are overwritten, there is no need to
call dired-insert-set-properties.

dired-filter.el Outdated Show resolved Hide resolved
dired-filter.el Outdated Show resolved Hide resolved
dired-filter.el Outdated Show resolved Hide resolved
(collapsed (save-excursion
(beginning-of-line)
(get-text-property (point) 'dired-filter-group-collapsed)))
(beg (save-excursion
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that you got rid of this!

@@ -44,6 +44,43 @@
:group 'dired
:prefix "dired-hacks-")

(defun dired-utils-fillin-invisible-property (beg end symbol)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two functions look generic enough (if we also pass the property name as arg) to be included in some other package (s.el?). We can leave them here for now but I know I've written similar code before.

They would be quite useful in Emacs itself as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I got the idea from font-lock-fillin-text-property.

@Fuco1
Copy link
Owner

Fuco1 commented Sep 22, 2018

I wanted to add "Comment" review not "Request changes" as not all are requests for changes (only 3 or so).

Good job overall, this is a very nice improvement!

Copy link
Owner

@Fuco1 Fuco1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice batch! I only have two small things and I think we're good to go.

dired-filter.el Outdated Show resolved Hide resolved
dired-filter.el Outdated Show resolved Hide resolved
@nnicandro
Copy link
Contributor Author

I just realized that collapsing filter groups didn't work with subtrees or the collapse feature. These last two commits should do the trick.

The main idea is to take advantage of
`add-to-invisibility-spec` and
`remove-from-invisibility-spec` when toggling the filter
group instead of overwriting the invisible property.

* Add the functions `dired-utils-fillin-invisible-property`
  and `dired-utils-remove-invisibile-property` that adds or
  removes a specific invisible property while preserving
  any current value of the invisible property according to
  the semantics defined in `buffer-invisibility-spec`.

* Add a new header property,
  `dired-filter-group-invisible-property` which stores the
  invisible property that is used when toggling the filter
  group. The property is formed from the name of the group
  and the current `point` at which the header will be
  inserted. This is so that filter groups with the same
  name but in different subdirectories have unique
  invisible properties.

* Combine making and inserting a group header into the
  function `dired-filter-group--insert-header`. Since we
  require the `point` of insertion for
  `dired-filter-group-invisible-property` it makes sense to
  combine the two operations instead of assuming that when
  `dired-filter-group--make-header` is called it is at the
  insertion point.

* Add the function
  `dired-filter-group--fillin-invisible-property` to add
  the invisible properties for the groups when they are
  inserted in `dired-filter-group--apply`.

* When `dired-filter-group-toggle-header` is called, add or
  remove the corresponding invisible property of the group
  header from the `buffer-invisibility-spec`.
* Use `dired-utils-(fillin|remove)-invisible-property` when
  updating the narrowed state since these preserve any
  invisible text properties already present. This allows us
  to remove the call to `dired-insert-set-properties`
* `dired-filter-group--make-header` now takes an additional
  argument to specify the invisible property for collapsing
  and expanding the header. This property is added as the
  `dired-filter-group-invisible-property` of the returned
  header text.
* Ensure that invisible text properties are updated for
  the line when replacing it so that we integrate with
  filter groups.
This function constructs the symbol associated to a filter
group for expanding and collapsing the group.
@nnicandro
Copy link
Contributor Author

Sorry for taking so long to get back to this. I've made all the changes you've requested. Let me know if there is anything else.

@Fuco1 Fuco1 assigned Fuco1 and unassigned Fuco1 Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants