Skip to content

Allow saving whole session with all workspaces#104

Draft
cedryc wants to merge 7 commits intoJonnyHaystack:masterfrom
cedryc:master
Draft

Allow saving whole session with all workspaces#104
cedryc wants to merge 7 commits intoJonnyHaystack:masterfrom
cedryc:master

Conversation

@cedryc
Copy link

@cedryc cedryc commented Jun 6, 2021

Work in progress

Made a pull request for this feature:

  • i3-resurrect save/restore --session to save whole session
  • i3-resurrect save/restore --workspace list-of-workspaces for saving multiples workspaces
  • i3-resurrect restore --session --clear for deleting active workspaces before restoring.

This implementation use subdir for saving profiles instead of files, to allow saving multiples workspaces or whole session profile

this may be usefull for some people

@cedryc cedryc marked this pull request as draft June 6, 2021 14:53
@cedryc cedryc closed this Jun 6, 2021
@cedryc cedryc changed the title Master Allow saving whole session with all workspaces #84 Jun 6, 2021
@cedryc cedryc changed the title Allow saving whole session with all workspaces #84 Allow saving whole session with all workspaces Jun 6, 2021
@cedryc cedryc reopened this Jun 6, 2021
Copy link
Owner

@JonnyHaystack JonnyHaystack left a comment

Choose a reason for hiding this comment

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

I reviewed some of this and have left some comments, but it's really a lot so it's pretty hard. Appreciate your work though. If you can keep the changes especially to the interface as minimal as possible then that would make things easier.

from . import util

DEFAULT_DIRECTORY = config.get('directory', '~/.i3/i3-resurrect/')
DEFAULT_DIRECTORY = config.get('directory', '~/.config/i3-resurrect/')
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you change this?

Copy link
Author

Choose a reason for hiding this comment

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

Ya it's not relevant here, so removed.

for window_id in window_ids:
xdo_unmap_window(window_id)
if target in ('clean', 'force'):
keept_windows = clean_workspace(layout, saved_programs, normal_windows, target)
Copy link
Owner

Choose a reason for hiding this comment

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

Don't understand this variable name. What is keept?

Copy link
Author

Choose a reason for hiding this comment

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

That represent windows that are preserved on the layout. Changed variable name to preserved_windows.

keept_windows = []
saved_windows = treeutils.get_leaves(layout)
for saved_program in saved_programs:
# print(saved_window)
Copy link
Owner

Choose a reason for hiding this comment

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

This line needs to be removed

Copy link
Author

Choose a reason for hiding this comment

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

Removed

directory = Path(expandvars(directory)).expanduser()
if profile is not None:
directory = directory / 'profiles'
directory = directory / profile
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you change this? That's not how the directory structure is

Copy link
Author

Choose a reason for hiding this comment

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

Yes is a personal preference, that was easier for me to implement the feature with profiles stored in dirs.This way, the directory name is the name of the profile. Also allow saving multiple workspace in the same profile name. So session profiles can be handled the same way as session without profile name.

"""
List saved workspaces or profiles.
"""
# TODO: list workspaces in profiles
Copy link
Owner

Choose a reason for hiding this comment

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

This is something that worked before. Please don't break existing functionality in a PR.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in PR update

README.md Outdated
--layout-only Only restore layout.
--programs-only Only restore running programs.
-c, --clean Move program that are not part of the workspace
layout to the scratchpad.
Copy link
Owner

Choose a reason for hiding this comment

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

Cool idea 😄

README.md Outdated
layout to the scratchpad.

-F, --force Close program that are not part of the workspace
layout before restore it.
Copy link
Owner

Choose a reason for hiding this comment

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

I notice both of these help texts have a big space in the middle because of how you've written them in the code, so you'll need to fix that.

Save i3 workspace(s) layout(s) or whole session and running programs to a file.

WORKSPACES are the workspaces to save.
[default: current workspace]
Copy link
Owner

Choose a reason for hiding this comment

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

What's up with this comment?

@click.option('--workspace', '-w',
help='The workspace to save.\n[default: current workspace]')
is_flag=True,
help='The workspaces to save.\n This can either be a name or the number of a workspace.')
Copy link
Owner

Choose a reason for hiding this comment

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

Why are you making this a flag?

Copy link
Author

Choose a reason for hiding this comment

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

To allow saving multiples workspaces at once. Workspaces name or number are parsed as arguments

@click.option('--workspace', '-w',
help='The workspace to restore.\n[default: current workspace]')
is_flag=True,
help='The WORKSPACES to restore.\nThis can either be a name or the number of a workspace.')
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't make sense that you're making this a flag but also claiming that it takes a list of workspaces which it doesn't...

Copy link
Author

Choose a reason for hiding this comment

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

Ya, changed the comment..

@TheBlackArroVV
Copy link

Would this feature be finished in the future?

@cedryc
Copy link
Author

cedryc commented Jun 27, 2022

Would this feature be finished in the future?

I made some changes on this feature in my repo, may need some further testing...

@Thaodan
Copy link

Thaodan commented Nov 6, 2023

When would the program to be ran to save the session? Is there a hook to hook into to be called before the session ends.
Same for restoring them.

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

Successfully merging this pull request may close these issues.

4 participants