Feature/oni sessions #2479

akinsho · 2018-07-30T07:23:21Z

Finally had a go at implementing this after the conversation on discord a while ago re. suggested implementation @bryphe (hopefully this is in line with what you wanted) re. multiplexing and putting the methods on the oni editor.

Currently this adds a side bar pane with a list of available sessions these are saved to a default directory HOME/.config/oni/sessions under the name a user inputs as a vim file. Selecting the input option adds an input which is the name we use for the session.

The list is gotten by reading the dir and getting all the files saved and on selecting one it call so ${session.file}

Outstanding

This reverts commit 97f0c61.

codecov · 2018-07-30T08:06:57Z

Codecov Report

Merging #2479 into master will decrease coverage by 0.66%.
The diff coverage is 49.13%.

@@            Coverage Diff            @@
##           master   #2479      +/-   ##
=========================================
- Coverage   44.26%   43.6%   -0.67%     
=========================================
  Files         345     351       +6     
  Lines       13933   14176     +243     
  Branches     1829    1845      +16     
=========================================
+ Hits         6168    6181      +13     
- Misses       7525    7758     +233     
+ Partials      240     237       -3

Impacted Files	Coverage Δ
ui-tests/mocks/Sidebar.ts	`100% <ø> (ø)`	⬆️
...src/Services/Configuration/DefaultConfiguration.ts	`87.5% <ø> (ø)`	⬆️
browser/src/Input/KeyBindings.ts	`1.86% <0%> (-0.04%)`	⬇️
browser/src/PersistentStore.ts	`30.43% <0%> (-2.9%)`	⬇️
browser/src/App.ts	`8.98% <0%> (-4.46%)`	⬇️
browser/src/neovim/NeovimInstance.ts	`5.55% <0%> (-6.98%)`	⬇️
browser/src/Editor/NeovimEditor/NeovimEditor.tsx	`9.09% <0%> (-0.28%)`	⬇️
...src/Services/VersionControl/VersionControlView.tsx	`73.01% <100%> (ø)`	⬆️
...rowser/src/UI/components/VersionControl/Staged.tsx	`96.15% <100%> (ø)`	⬆️
...owser/src/UI/components/VersionControl/Commits.tsx	`94.11% <100%> (ø)`	⬆️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fd82f3...f68b061. Read the comment docs.

t

…ni-sessions

CrossR · 2018-07-30T11:04:47Z

browser/src/Services/Sessions/SessionManager.ts

+    }
+
+    public get sessionsDir() {
+        return path.join(getUserHome(), ".config", "oni", "sessions")


I think you'll want to use getUserConfigFolderPath here, which gets the oni folder directly.

On Windows the oni folder is in %APPDATA%\oni versus ~/.config/oni.
May also help testing.

oni/browser/src/Services/Configuration/UserConfiguration.ts

Lines 27 to 40 in e46c720

export const getUserConfigFolderPath = (): string => {

const configFileFromEnv = process.env["ONI_CONFIG_FILE"] as string // tslint:disable-line

Log.info("$env:ONI_CONFIG_FILE: " + configFileFromEnv)

if (configFileFromEnv) {

const configDir = path.dirname(configFileFromEnv)

Log.info("getUserConfigFolderPath - path overridden by environment variable: " + configDir)

return configDir

}

return Platform.isWindows()

? path.join(Platform.getUserHome(), "oni")

: path.join(Platform.getUserHome(), ".config/oni") // XDG-compliant

}

TalAmuyal · 2018-07-30T16:01:00Z

Looking forward to that feature!

…

On Mon, Jul 30, 2018, 14:04 Ryan C ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In browser/src/Services/Sessions/SessionManager.ts <#2479 (comment)>: > + private _sidebarManager: SidebarManager, + private _commands: Commands.Api, + ) { + fs.ensureDirSync(this.sessionsDir) + this._sidebarManager.add( + "save", + new SessionsPane({ store: this._store, commands: this._commands }), + ) + } + + public get sessions() { + return this._store.getState().sessions + } + + public get sessionsDir() { + return path.join(getUserHome(), ".config", "oni", "sessions") I think you'll want to use getUserConfigFolderPath here, which gets the oni folder directly. On Windows the oni folder is in %APPDATA%\oni versus ~/.config/oni. May also help testing. https://github.com/onivim/oni/blob/e46c720fc82f1466347016bc0916a8d744c94793/browser/src/Services/Configuration/UserConfiguration.ts#L27-L40 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2479 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE3IjShabgZSOL9gBGkMDrsCjqxMkU25ks5uLuhTgaJpZM4Vl52L> .

move section title to general location add delete session functionality

fix delete session bug causing reappearances

… session

ile

remove need for instantiation, use done callback

a large refactor of all observables is going to be required to upgrade redux observable so seems counter productive to write tests that will need to be re-written entirely once that is done

render session sidebar item second

…ni-sessions

akinsho · 2018-08-21T22:27:55Z

@CrossR @TalAmuyal would appreciate a review on this as I think its ready to go now.
The functionality is intentionally pretty minimal at this stage, its essentially a wrapper around calling mksession and source session, session files default to being saved in ~/config/oni/sessions/ but can be configured. Its under an experimental flag by default also.

CrossR

Given it a quick test, not 100% on the features added so this is what I've tested:

Opening a few files in splits/tabs, saving a session.
Restoring a session.
Changing the splits/tabs about, and the restoring again to check its saved.
Deleting saved sessions.

I'm getting some weirdness, ie I'll close with a single tab and open with 2 tabs of a file, but that doesn't seem to be an Oni issue, more the session file has 2 calls to tabedit for whatever reason.... I'm a bit of a sessions noob so who knows. The Oni side works great, its more me not quite getting the nvim side.

One thing I did notice is that it seems to constantly save, is that intended? I would have assumed it would only save after some changes are made, or the cursor moves etc, but it seems to save constantly when the cursor is in the buffer? If that's default behaviour, just ignore me.

For a later PR, bringing over the undo feature would be nice, or a confirmation of delete (or both), but thats just for a later PR.

akinsho · 2018-08-21T23:06:02Z

@CrossR thanks for the review 👍 ✨,
Re. functionality thats actually all it does for the time being 😆, it relies heavily on the sessiopts that a user has set so things like what tabs are where etc. all come from a users init.vim seemed the cleanest way for a vim user though for new users of vim it might eventually need some sort of default setting for sessionopts if neovim doesn't already have sensible defaults I've no idea what is set by default.

re saving all the time, its debounced 200ms and tbh listens to the bufenter event so shouldn't be calling mksession unless you are entering a buffer, when you say constantly save what was happening? calls to mkession in the console?

CrossR · 2018-08-21T23:19:09Z

re saving all the time, its debounced 200ms and tbh listens to the bufenter event so shouldn't be calling mksession unless you are entering a buffer, when you say constantly save what was happening? calls to mkession in the console?

If I sit with the cursor in the buffer and the sidebar opened, I see the "Last modified" never changes from 0 seconds, and if I watch the file in the sessions folder, I can see the modified time changing even if I'm not moving or doing anything in the buffer, other than just having the cursor sat there in normal mode.

akinsho · 2018-08-21T23:25:52Z

Ahh that's a result of the way that I render the time string which I guess can be confusing basically the time shown is the time since it was last modified so basically if its a very recent change say a completely new session then it will say last modified 10s then 15s as time goes on.

The idea was that you could see at a glance the last time you used a session so if one is months old you might think to delete it, wasn't strictly necessary just something I though would be nice to have, the actual time is static a prev. commit had it rendering as a full date string but it just seemed a bit less useful since the core info I was trying to convey was how recently it was used

akinsho · 2018-08-21T23:43:17Z

Actually just re-read your comment if the file in the filesystem is changing its modified time every second thats not intended but if oni is getting and losing focus repeatedly that would trigger a bufEnter, I just had a look at the dir locally and all the session files have the right mtime's none are changing until I switch buffers around 😕

akinsho · 2018-08-22T07:16:37Z

Going to merge this now and can keep an eye on how often the session is being saved and see if I can reproduce the constant save issue.

feltech · 2018-08-24T09:47:49Z

I had a go at enabling this. Previously I had been using vim-session, but that is rather glitchy in Oni.

Unfortunately I found with Oni's session support enabled I seemed to randomly lose unsaved work. I would look away for a minute or two, then when I came back the text had reverted to what's saved on disk.

I haven't had a chance to look into it further. I'm using Oni as my main code editor at work (in python), so can't spend much time trying to debug it here.

akinsho · 2018-08-24T09:51:25Z

@feltech thanks or trying out the functionality 👍, you are right I've noted the issue but havent had time to log it properly, it probably relates to what @CrossR was noticing re. the updating not sure what or how or whats going on but will debug once i get a chance

akinsho · 2018-08-28T09:58:35Z

Hmm seems the bug here persists with this PR disabled so might actually be due to something else 😭, not sure what would cause a buffer to revert to an earlier version maybe a change thats affecting the prettier autoformatting on save 😕

akinsho added 10 commits July 29, 2018 15:07

add session manager, store and sidebar pane

d1227ae

hook up session services to app

ec3d01d

move side effects into epics

3668108

add creation and cancel creation actions

ce8600e

add restore session epic and remove console.log

815b618

await sessions in case of error

8558f10

add error handling to session functions

9ba6795

Revert "Refocus previously open menu on reactivating Oni (onivim#2472)"

7bf2f0f

This reverts commit 97f0c61.

Revert "Refocus previously open menu on reactivating Oni (onivim#2472)"

8000

8a8e6dc

This reverts commit 97f0c61.

remove console.log

b5319bd

akinsho changed the title ~~Feature/oni sessions~~ [WIP] Feature/oni sessions Jul 30, 2018

akinsho added 2 commits July 30, 2018 08:32

remove unused props passed to session pane

7845291

add persist session command [WIP]

f989c83

akinsho added 2 commits July 30, 2018 10:15

Add current session action and update store with i

eded911

t

Merge branch 'master' of https://github.com/onivim/oni into feature/o…

34e9cea

…ni-sessions

CrossR reviewed Jul 30, 2018

View reviewed changes

akinsho added 12 commits July 30, 2018 17:37

use get user config add icons and some error handling

5d8e6d4

add unclick handlers for sessions

06ebb90

move section title to general location add delete session functionality

add title and toggling of sessions add on vim leave handler

9ed141b

fix lint errors

fb6116f

refactor epics in preparation for 1.0 and rxjs 6
10000

aff0d1e

update snapshot

71435af

add bufdo bd command prior to restoring session

197f932

fix delete session bug causing reappearances

create separate close all buffers method

c23b0d6

remove update session method

fe368f3

Add audit time to update current session

9ceb782

add close all buffers method and use persist session action in update…

9d3ef81

… session

Merge branch 'master' of https://github.com/onivim/oni

6f6464e

akinsho added 18 commits August 6, 2018 10:32

switch to adding updated at by checking mtime of f

7f395e5

ile

switch to storing sessions in persistent store

a925185

add delete functionality to persistent store and mock

65ab5dd

fix lint error

1a55c56

rename sessionName var to name for simplicity

94d8db5

add session manager initial test

87dc346

create path using path.join

8d4c1d1

add experimental config flag

54dd1be

update Oni mock and increase sessionmanager tests

ed6ef6d

add session store tests - [WIP]

7bdb53e

return simple session manager mock

a7df5a5

remove need for instantiation, use done callback

remove session store tests as redux observable api has changed

ee5766f

a large refactor of all observables is going to be required to upgrade redux observable so seems counter productive to write tests that will need to be re-written entirely once that is done

add user configurable session directory

cbf1901

tweak sidebar item styles

c8e7d01

update vim navigator to pass function to update selected item on click

42a20c8

render session sidebar item second

Merge branch 'master' of https://github.com/onivim/oni into feature/o…

2872bdf

…ni-sessions

fix lint error

4952ef0

fix broken tests

f68b061

akinsho changed the title ~~[WIP] Feature/oni sessions~~ Feature/oni sessions Aug 21, 2018

CrossR approved these changes Aug 21, 2018

View reviewed changes

akinsho merged commit e3f65e6 into onivim:master Aug 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/oni sessions #2479

Feature/oni sessions #2479

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	export const getUserConfigFolderPath = (): string => {
	const configFileFromEnv = process.env["ONI_CONFIG_FILE"] as string // tslint:disable-line
	Log.info("$env:ONI_CONFIG_FILE: " + configFileFromEnv)

	if (configFileFromEnv) {
	const configDir = path.dirname(configFileFromEnv)
	Log.info("getUserConfigFolderPath - path overridden by environment variable: " + configDir)
	return configDir
	}

	return Platform.isWindows()
	? path.join(Platform.getUserHome(), "oni")
	: path.join(Platform.getUserHome(), ".config/oni") // XDG-compliant
	}

Feature/oni sessions #2479

Feature/oni sessions #2479

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!