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

Copying custom splitter items #52

Closed
dima-lur opened this issue Apr 17, 2017 · 10 comments
Closed

Copying custom splitter items #52

dima-lur opened this issue Apr 17, 2017 · 10 comments
Milestone

Comments

@dima-lur
Copy link

dima-lur commented Apr 17, 2017

Hi.
Currently I develop my own splitter. I noticed one nasty feature when copying the panels contained in the splitter (Preferences-> Display-> ColumnsUI-> Layout-> Copy panel). The copy_splitter_item function from the config_layout.cpp creates an instance of splitter_item_full_v2_impl_t to copy to the clipboard. However, it seems more appropriate to create a copy of the child panel using the get_panel interface of the class splitter_window, because other commands like "Paste panel" or "Move up" use insert_panel, get_panel, etc.
This is necessary so that I can copy the panel that has additional attributes if I inherit from splitter_item_t

@dima-lur
Copy link
Author

After some research I am ready to admit that unfortunately, it is impossible to amend the existing code without violating the compatibility with other splitters. Is it true?

@reupen
Copy link
Owner

reupen commented Apr 18, 2017

Hi,

You raise a good point. From looking, the caller assumes ownership of the object returned by splitter_window::get_panel(), so there is no problem there (as in it must be a new object).

The risk I can see is that the returned splitter_item_t instance has a window_ptr and pasting it results in two splitter panels having a child panel with the same window_ptr. The risk is minimal, though, since we're dealing with a non-live layout (for live editing it would be worse).

The problem could be fixed with a new subclass of either splitter_item_t or splitter_window. Backwards compatibility shouldn't be a major problem since there can be multiple code paths in Columns UI (and you can use the new interface for your splitter panel). I'm not sure what the best approach would be though; I would probably need to look at the code base more closely to see how exactly the window_ptr of splitter_item_t is used.

What were the concerns you had? I may have missed something.

Thanks

@reupen
Copy link
Owner

reupen commented Apr 18, 2017

It's also worth noting that two of three of the built-in splitters do not call splitter_item_t::get_window_ptr() in splitter_window::insert_panel().

@dima-lur
Copy link
Author

Yes, I see. But: copying in layout preferences calls copy_splitter_item which uses splitter_item_full_v2_t as clipboard item. I want to use my own subclass of splitter_item_full_v2_t in clipboard. I tried to change code in copy_item that it calls get_panel (for compatibility with splitter_window interface) to create my subclass object. It seems OK, but next in paste_item there is fix_paste_item (it fixes pasting non-single-instance panels) that recopy splitter item again with splitter_item_full_v2_t class.

@dima-lur
Copy link
Author

Main problem is what right way to get exact copy of my subclass object without injecting any code in Columns UI project for backward compatibility purpose

@reupen
Copy link
Owner

reupen commented Apr 18, 2017

You can't. The code in Columns UI needs changing and possibly the API.

@dima-lur
Copy link
Author

I was afraid of this. Thanks for reply.

@reupen reupen added the api label Apr 23, 2017
@reupen reupen added this to the v0.6.0 milestone Dec 28, 2017
reupen added a commit to reupen/columns_ui-sdk that referenced this issue Jan 26, 2018
Part of reupen/columns_ui#52

This adds a new splitter item, that can store additional data that doesn't fit into the standard variables provided in the existing splitter items.

This also removes splitter_item_t::set(), as it was untested and not properly implemented.
reupen added a commit that referenced this issue Jan 26, 2018
Resolves #52

Improves how copying and pasting panels in the layout preferences page works. Additional data can now be stored in splitter items using the new uie::splitter_item_full_v3_t class in the SDK.

This work will also make it possible to use the Windows clipboard for copying and pasting panels.
@reupen
Copy link
Owner

reupen commented Jan 26, 2018

I came up with a solution for this (without rewriting the API) in these PRs:

#71
reupen/columns_ui-sdk#5

Essentially, the solution requires you to modify your splitter_window::get_panel() implementation to return an instance of uie::splitter_item_full_v3_t. You should write out the extra attributes in get_extra_data(), and make get_extra_data_format_id() return a unique GUID.

And your splitter_window::insert_panel() and splitter_window::replace_panel() implementations will need to query the splitter item for the uie::splitter_item_full_v3_t interface, and, if successful, check the format ID and read the extra data.

There are some additional notes in splitter.h in the SDK. You can use uie::splitter_item_full_v3_impl_t instead of implementing uie::splitter_item_full_v3_t from scratch. You can also ignore the data members of uie::splitter_item_full_v3_t that you don't need.

Feel free to try it out and let me know how you get on. I'll probably be making copying and pasting panels use the real clipboard as well.

@dima-lur
Copy link
Author

dima-lur commented Feb 14, 2018

Thank you very much for this solution. I've tried it and it works like a charm =)

@reupen
Copy link
Owner

reupen commented Feb 17, 2018

Great! And no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants