Skip to content

Add var to filter saved frames on bookmark-frames #24

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gagbo
Copy link

@gagbo gagbo commented Nov 30, 2020

This allows for example to not save childframes like the ones created by
company-box and posframe

Draft implementation because

  • I am not sure about the variable name
  • I am not sure about it's default behaviour (remove childframes or 100% passthrough ?)
  • I am not sure about the implementation (how do we check for a childframe ?)

@alphapapa
Copy link
Owner

It would probably be better to follow the example of burly-frameset-filter-alist and frameset-filter-alist than to define a predicate that only tests a single thing (which would make it difficult for users to extend its behavior).

Also, FYI, the following forms are equivalent:

(unless (frame-parameter frame 'parent-frame)
    t)

(not (frame-parameter frame 'parent-frame))

@gagbo
Copy link
Author

gagbo commented Dec 3, 2020

It would probably be better to follow the example of burly-frameset-filter-alist and frameset-filter-alist than to define a predicate that only tests a single thing (which would make it difficult for users to extend its behavior).

I wast thinking about making the predicate a lambda so anyone can write and combine conditions. The issue with burly-frameset-filter-alist is that it's used only to choose whether we should save a given frame-parameter, so we know that we only get a list of symbols and have to answer yes/no.

When choosing if we should save the whole frame there are others parameters that might be relevant:

  • maybe someone doesn't want to save small frames,
  • maybe they don't want to save a frame if it has only 1 tab,
  • maybe they don't want to save a frame if all tabs have the same name,
  • maybe they don't want to save a frame if it only showed special buffers

And I couldn't find a way to express all those posibilities using anything else than a lambda. Using a filter-alist would mean writing a complex tagging system to make for all the possible choices a enduser would like to make.

(unless (frame-parameter frame 'parent-frame)
     t)

I knew it was bad somehow, there were too many negations in my head to write it correctly. Thanks :)

@alphapapa
Copy link
Owner

Ok, if you think those examples might happen in actual use, then we can use a predicate instead. Specifically, we should probably use a list of "negatory" predicates, and each frame should be tested against those predicates, and if any predicate returns non-nil, the frame should be rejected. What do you think? Thanks.

@gagbo
Copy link
Author

gagbo commented Dec 3, 2020

The childframe part is pretty obvious since it happens with all posframe/childframe related packages (company-box, eldoc-box, ivy-posframe, its very likely helm variant, flycheck-posframe...)

Not saving if the frame has a single tab could happen if I only want to save "complex" frame configurations automatically on a hook, and I know single tab frames aren't worth it.

I'm planning to make yet another buffer grouping plugin (only difference with bufler would be to be able to assign multiple tags to a single buffer), and if I only have special buffers left in a "tag" I don't want to save the window configuration associated with this tag.

For these cases I thought that leaving the possibility of writing a custom predicate function would be better.

I can change the variable to be a list of "ignore" predicates for sure, and provide a small list of what I think would be common examples (i.e. childframes, and a "factory" that builds "ignore if placeholder frame-parameter is set" predicates)

@alphapapa
Copy link
Owner

I'm planning to make yet another buffer grouping plugin (only difference with bufler would be to be able to assign multiple tags to a single buffer), and if I only have special buffers left in a "tag" I don't want to save the window configuration associated with this tag.

If that's really the only difference from Bufler, why not open an issue on Bufler's repo and we can talk about implementing that paradigm in Bufler? I guess it might be as "easy" as making a version of seq-group-by that doesn't consume matches.

I can change the variable to be a list of "ignore" predicates for sure, and provide a small list of what I think would be common examples (i.e. childframes, and a "factory" that builds "ignore if placeholder frame-parameter is set" predicates)

That sounds good. Thanks.

@alphapapa
Copy link
Owner

Regarding that idea for Bufler, see alphapapa/bufler.el@d623605

@gagbo
Copy link
Author

gagbo commented Dec 4, 2020

If that's really the only difference from Bufler, why not open an issue on Bufler's repo and we can talk about implementing that paradigm in Bufler? I guess it might be as "easy" as making a version of seq-group-by that doesn't consume matches.

Sure, I was just afraid it would be too big of a change given the way defgroups was handled

@gagbo gagbo force-pushed the frame_filter_predicate branch 4 times, most recently from bd93b94 to 96ca4c4 Compare December 13, 2020 15:38
@gagbo gagbo marked this pull request as ready for review December 13, 2020 15:55
@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 6303aad to c6d6aae Compare June 21, 2021 08:45
@alphapapa alphapapa added the enhancement New feature or request label Jun 22, 2021
@gagbo gagbo force-pushed the frame_filter_predicate branch from 96ca4c4 to b837e52 Compare July 9, 2021 07:10
This allows for example to not save childframes like the ones created by
company-box and posframe

Co-authored-by: Adam Porter <adam@alphapapa.net>
@gagbo gagbo force-pushed the frame_filter_predicate branch from b837e52 to 4f53f91 Compare August 1, 2021 07:50
@alphapapa alphapapa self-assigned this Jun 8, 2023
@alphapapa alphapapa added this to the 0.4 milestone Jun 8, 2023
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

Successfully merging this pull request may close these issues.

2 participants