Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

[WIP] Bugfix/vcs deactivation race condition #2453

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

Conversation

akinsho
Copy link
Member

@akinsho akinsho commented Jul 24, 2018

Currently when switching workspaces if, switching to one without a vcs folder the deactivation function is called midway through the update branch indicator function currently causing the current provider to be set to null during the function execution.

This change add a promise queue using pqueue this allows more granular control than our current homegrown queue as it allows delaying function execution till the queue is idle it allows clearing the queue checking it and concurrency control

@akinsho akinsho changed the title Bugfix/deactivation race condition [WIP] Bugfix/deactivation race condition Jul 24, 2018
@codecov
Copy link

codecov bot commented Jul 24, 2018

Codecov Report

Merging #2453 into master will increase coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2453      +/-   ##
==========================================
+ Coverage   43.27%   43.27%   +<.01%     
==========================================
  Files         341      341              
  Lines       13425    13429       +4     
  Branches     1765     1765              
==========================================
+ Hits         5809     5811       +2     
- Misses       7340     7342       +2     
  Partials      276      276
Impacted Files Coverage Δ
.../Services/VersionControl/VersionControlManager.tsx 56.33% <33.33%> (-0.19%) ⬇️

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 ecbd4af...7e9ef86. Read the comment docs.

@akinsho akinsho changed the title [WIP] Bugfix/deactivation race condition Bugfix/deactivation race condition Jul 24, 2018
@akinsho akinsho changed the title Bugfix/deactivation race condition Bugfix/vcs deactivation race condition Jul 24, 2018
@akinsho akinsho changed the title Bugfix/vcs deactivation race condition [WIP] Bugfix/vcs deactivation race condition Jul 24, 2018
akinsho and others added 21 commits July 25, 2018 12:25
Thanks, @Akin909 :)
As mentioned in onivim#2344 buffer layers suffer from laggy rendering on scroll, on some investigation it seems the `auditTime` we use in the `shouldMeasure$` observable means that the information is not being updated in real time so scrolling quickly through a buffer causes indentlines or color highlights to appear in a *janky* fashion i.e. they are out of place.

Not sure what it was used for but I think because the information is required by ui pieces which need v. up to date data re window details we should allow things to be updated as often possible. This change greatly improves the rendering.

There are a couple of things which still cause the ui to be a bit janky:
* one is the initial buffer entering although this is arguably low priority 🤷‍♂️ - seems quite a lot happens there the first time and rendering is noticeably slower.
* folding is completely unhandled (not investigated yet)
* double line wrapping
* searching in a buffer which can change the position of the window doesn't seem to update the window positioning (any ideas @bryphe?) - i.e. searching in buffer with `?` or `/` can move the viewport towards the match(es) in this case window details passed to context don't seem to update
* initial load of a buffer similar to one
### Other additions:

* I also tweaked the indent lines to use a `bannedFiletypes` buffer as opposed to a whitelist aka as discussed with @CrossR it is now on for all filetypes except those which are blacklisted. @CrossR examples of where you would want a black list I think would be most vim plugin buffers like `peekabo` or `startify` or `nerdtree` etc. in these cases the indentlines rarely render desirably and are arguably not required.

* I render the first indent line which is the same behaviour as in `vscode` however i've added an option. `experimental.indentLines.skipFirst` which allows a user to avoid this behaviour if they would prefer
* In the command palette when the Explorer is selected there were two
options to create a new file.  One of them is actually the command to
create a new directory.
* So fix the incorrect name for that.
* Also harmonise the naming scheme of the other explorer actions.
* Basic code highlights.

* Check if the given language is valid, use auto highlight if not.

* Fix check.

* Set baseUrl as well.

Unrelated to code blocks, but fixes onivim#1653 and is a single line change in the options I've added.

* Bump marked types package number.

* Move style sheet to only markdown preview.

* Swap theme to option.

* Fix review comment from previous PR.

* Fix nohighlights.

* Update tests for Markdown preview.

* Fix lint error.

* Add config option for syntax highlights.
* add unstage command and binding

* add getLogs function to provider and render git logs

* make latest commit selectable

* add uncommit functionality

* add help ui improvements and read in current bindings

* pass in commands and default to global var

* fix tests to match new git logs interface

* add test to assert help prompt is shown

* add test for help component

* remove unused project root argument

* add scrollability to the list no entire component

* disable new binding if commiting

* add empty fileChanged event dispatch to uncommit a

ction

* add wrap in brackets and extra commands to help co

mponent

* simplify commit submission handling

* create refresh function

* refresh in the finally method of commitFile functi

on

* re-add component did mount function and call get s

tatus

* refresh data in vcs pane on dir change and file status even if no t visible

* update help component test

* consolidate error message handling

* fix help test greater than assertion
* Command to find current buffer in Explorer and select it (onivim#1567)

Add command palette option "Explorer: Locate Current Buffer" that will
expand directories down to the current buffer, then select the
corresponding file/directory.

* Only dispatch success action if/when selection changed as expected

Prevent sending pointless `SELECT_FILE_SUCCESS` actions if we're not
actually in the process of selecting a file (or haven't selected the
file we expect yet).

* Open Explorer if it's not visible when executing Locate Current Buffer.

* Keep `registerCommand` calls together in the `index.tsx` and unify
their style.
* Use the `SidebarManager` to first ensure the Explorer is shown, before
then locating the file in the `ExplorerSplit`.

* Remove duplicated `nodePath` function in favour of `getPathForNode`

* Fix sidebar hiding when locating file in Explorer

* Strangely, `SidebarManager.setActiveEntry` will close the sidebar if
the Explorer is already open and you try to open it again.
* So only trigger opening the Explorer if it's not already visible.

* tslint: alphabetize imports

* Fix test conflict

* `epicMiddleware.replaceEpic` internally dispatches a
`@@redux-observable/EPIC_END` action, which gets saved to the
`mockStore`'s
actions list. Every test case's `afterEach` thus adds an additional
action to this actions list. This pollutes all subsequent stores created
via the `mockStore` factory, which is odd.
* This was accounted for in the YANK_AND_PASTE_EPICS test by assuming
"an init action is sent first", which is not the case.
* So move the setup that is only for the SET_ROOT_DIRECTORY test inside
it's own `describe` block, remove the unnecessary `replaceEpic` call,
and fix the test for expected number of actions.

* Add `selectFileReducer` tests

* Add `selectFileEpic` tests

* Minor test tidying

* Add notificationEpic SELECT_FILE_FAIL test

* Add VimNavigator and ExplorerView tests

* Remove unused imports in test + tidy

* Add ExplorerSplit tests

* Add Locate Buffer integration test

* tslint: alphabetise imports

* Possibly fix SELECT_FILE on OSX + add logging to isolate problem

* Fix Explorer.LocateBufferTest for symlinked tempdir (i.e. OSX)

* Resolve symlinks before testing if selection is a descendant of workspace

* Add a hook to `IFileSystem` for `realpath`.
* Use that to resolve symlinks in the `selectFileEpic`.
* Also be slightly more robust and use `path.relative` to test if file
path is a descendent of workspace, rather than `startsWith`.
* Revert changes to the `Explorer.LocateBufferTest` from previous
attempt to fix for OSX.
* Leave in some logging on the expectation that OSX will still fail.

* Fix ExplorerStore tests

* Harmonise `realPath` => `realpath` so that mocks work.
* Monkey patch `MemoryFileSystem` to have a trivial `realpath` function.
* Make `selectFileEpic` tests asynchronous.

* Put symlink-less file to select in the store (for OSX)

* Use temporary workspace for QuickOpenTest

* `Explorer.LocateBufferTest` changes the workspace to a
temporary workspace, but `QuickOpenTest` wanted the Oni source directory.
* So use a temporary workspace for `QuickOpenTest` as well and commonise
the construction of such workspaces into `Common.ts`.
* Also removed some debug logging.

* Fix Neovim.CallOniCommands test

* There is a known bug in the typescript language server client (e.g.onivim#1588)
* So don't create files that trigger the language server in a test that
has nothing to do with it.

* Attempt to fix race in PrettierPluginTest on OSX

* Revert "Attempt to fix race in PrettierPluginTest on OSX"

This reverts commit 22a6424.

* Use temporary workspace for PrettierPluginTest

* Another attempt to get the test passing on OSX.  Hoping that the
workspace is just dirtied by previous tests.
* Removed pointless text from test `File.ts` to make this work.

* Move UI elment assertion of PrettierPluginTest to the end

Hopefully the other assertions will give a better clue what is failing
on OSX.

* Add logging to isolate issue with PrettierPluginTest on OSX

* Fix logging to isolate issue with PrettierPluginTest on OSX

* More logging to isolate issue with PrettierPluginTest on OSX

* Wait for status bar element in PrettierPluginTest on OSX

* Allow failure to get window count in test afterEach

* Fix waiting for status bar element in PrettierPluginTest on OSX

* Remove before/afterEach from runInProcTest to enable retries

* Add some logs when oni is close after test

* Add a couple of mocha flags that may stabalise tests

* Remove debug logs + tidy

* Tolerate failure to get window count when stopping Oni

* Add a retry to WindowsInstallerTests

* Revert "Add a retry to WindowsInstallerTests"

This reverts commit d6b4a29.

* Remove pointless mocha flags
* Add a `SELECT_FILE` action as the last step in `CREATE_NODE_COMMIT` so
that files created in the explorer are scrolled and highlighted.
* `SELECT_FILE` has the same effect as `EXPAND_DIRECTORY` + `REFRESH`,
so no need to include those as well.
* Updated unit tests.
* If the command menu is open and you alt-tab away from Oni, then
alt-tab back again, the menu remains open but no longer has cursor
focus.
* So ensure focus is not lost on reactivating Oni by not overwriting the
saved focus stack.
Reverts onivim#2472

@CrossR @bryphe this awesome bugfix PR unfortunately breaks a users ability to to navigate too and from the sidebar using keyboard bindings, in my excitement to have this annoying issue fixed I totally missed this bug till just now.
…im#2473)

Move API to oni-api and use it in preparation for pluginization
* Revert "Refocus previously open menu on reactivating Oni (onivim#2472)"

This reverts commit 97f0c61.

* add initial git blame layer implementation

* make component absolute and pass position to component

* add cursor to layer context and use it render positional blame

* render blame inline if possible otherwise render above

* format blame date str and add format date to utils

* change ordering of blame message

* format time as time since and update styles for blame

* separate out inline and hover styles

* rename inline var

* add out of bounds checking

* add more error handling to out of bounds check dar

ken tooltip color

* darken text in blame

* fix lint error in blame layer

* fix existing broken tests

* add config options and experimental flag
add ability to run in manual mode require a binding to be hit to show the blame

* fix lint errors

* fix cursor line variants naming
add comments to explain, also fix out of bounds function

* fix comment change order of updateBlameCall

* use get derived props from state to prevent initial render flicker
pass in font-famil

* switch back to concurrently for start
query only a single line at a time and update state with next props

* remove unsed var in blame container

* prevent layer from rendering if vcs.blame is not enabled

* add recursive truncation of summary message

* add check if shortened all the way to the bottom just render the author and time

* fix comment typo

* only append ellipsis is shortened summary has content

* remove hover style as it is hard to manage improve fit checking

* position blame in first empty white space above is cant fit

* remove unused arguments

* increase default timeout, add exiting to transitionStyles

* move opacity in hope of making animation more pronounced

* check line position at start of reset timer matches current one before showing blame

* fix position related crash

* prettier fix

* mount the component ffs, add more tests

* moar test for blame layer

* fix failing test

* add overflow component and re-arrange comments

* fix lint errors

* add component did catch method
use words to calculate if truncation should happen
append only if truncated, use proper truncation symbol

* fix broken truncation assertion

* change check in git blame layer to use currentLine content

* fix configuration to use _oni.configuration
Fix local building issue
* add key to bookmarks pane

* add key to learning pane

* add keys to window splits

* add key to plugins sidebar items

* use promise.all and move loading to view

* test 2

* re-add missing await to commit function

* re-add loading to pane

* add files to commit to state and use that to check if loading

* fix version control store tests
akinsho and others added 18 commits August 19, 2018 17:20
This PR fixes onivim#2114 and a few issues with the buffer scroll bar

The first issue relates to onivim#2114 which was the fact the the buffer bar lagged behind if the window was resized. The specific change that fixes this is changing the position from `absolute` to `fixed`

The other issue was that the error markers weren't rendering correctly i.e. only one error marker was shown in the buffer because the `uniqBy` function was filtering by Id but actually there was no id prop. Ive changed this to filter by line so the scroll bar just shows errors by line, meaning you now get multiple errors in the scroll bar based on position

Also took the opportunity to change the components to styled components with dynamic attrs and added keys to the error components
…le (onivim#2511)

Changes related to the file explorer spacing as explained in onivim#2510 
This change hopefully makes the file explorer easier to read. 

1. Slightly reduced spacing between each element in the explorer. 
2. Reduced space between the caret/icon and the text.
3. Doubled the indentation amount.
* update regex to look for word boundaries, update f

ont family if config changes

* remove unnecessary word boundaries from color rege

x

* vertically center text inside color highlight add highlight component
with component did catch if error remove higlights

* add a background component to prevent underlying color from bleeding through

* replace data id on new component
* add session manager, store and sidebar pane

* hook up session services to app

* move side effects into epics

* add creation and cancel creation actions

* add restore session epic and remove console.log

* await sessions in case of error

* add error handling to session functions

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

This reverts commit 97f0c61.

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

This reverts commit 97f0c61.

* remove console.log

* remove unused props passed to session pane

* add persist session command [WIP]

* Add current session action and update store with i

t

* use get user config add icons and some error handling

* add unclick handlers for sessions
move section title to general location
add delete session functionality

* add title and toggling of sessions add on vim leave handler

* fix lint errors

* refactor epics in preparation for 1.0 and rxjs 6

* update snapshot

* add bufdo bd command prior to restoring session
fix delete session bug causing reappearances

* create separate close all buffers method

* remove update session method

* Add audit time to update current session

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

* add restore session error action and complete action

* add console log for debugging return metadata directly

* add error handling to git blame function

* reduce persist audit time make ids in session.tsx readonly

* comment out console.log

* check neovim for current session when updating
this ensures the session is valid
added getCurrentSession method which checks vims v:this_session var

* fix lint errors

* add tests for sessions component and mock for neovim instance

* switch generic input check to specific text input view check

* add update timestamp functionality so these are more accurate

* switch to adding updated at by checking mtime of f

ile

* switch to storing sessions in persistent store

* add delete functionality to persistent store and mock

* fix lint error

* rename sessionName var to name for simplicity

* add session manager initial test

* create path using path.join

* add experimental config flag

* update Oni mock and increase sessionmanager tests

* add session store tests - [WIP]

* return simple session manager mock
remove need for instantiation, use done callback

* remove session store tests as redux observable api has changed
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

* tweak sidebar item styles

* update vim navigator to pass function to update selected item on click
render session sidebar item second

* fix lint error

* fix broken tests
This is a fix for some of the issues raised in onivim#2414. One thing to note - there was some question as to whether win32yank works for everyone. I did find a possible workaround for that, if it's still an issue:

```
const textToPaste = clipboard.readText()
const sanitizedTextLines = replaceAll(textToPaste, { "'": "''" })
await neovimInstance.command("let @+='" + sanitizedTextLines + "'")
```

I don't have a windows box to test this on, so perhaps we can add this code to `NeovimEditorCommands. pasteContents` later if needed.
This PR will fix onivim#1413 and onivim#1459 it uses the handle input method on the buffer layers to define a custom input handler for the welcome layer which is fairly simple i.e. `j` and `k` for navigation and `<enter>` to select, this PR also adds a method to the oni api called `getActiveSection` which returns a string representing the active section of the editor e.g. the particular sidebar element or the menu or commandline

Outstanding

- [x] Hide cursor when welcome is active
- [x] Fix welcome commands (some...)
- [x] Allow `enter` key passthrough
- [x] Add tests

~add sessions to welcome screen~ leave for another PR
As @josemarluedke mentioned [here](onivim#2515 (comment)), pasting in command line mode was broken with onivim#2515.
This PR adds `oni.editor.nextError` and `oni.editor.previousError` commands, as discussed in onivim#2514. I have mainly tested this with Typescript.
* create a helpers class to source the active section

* [WIP] add createWelcome function in editor

* implement an input handler for welcome buffer layer

* convert adhoc styles to styled components

* improve typing of commands
fix navigation of commands

* fix lint errors

* more lint fixes

* remove deprecated extend function

* add welcome layer test and fix types and typo

* add test of enter event

* remove welcome active boolean

* remove unused var

* add initial [WIP] ci test for welcome layer

* add new commands to Oni and use in welcome buffer
create common getBorder function

* remove z-index from neovim surface fix welcome tests

* add current layer is active check to see if layer should intercept input
move quick commands to the top of the welcome screen
fix getBorder function to return default border of same size

* revert vim navigator to master and move changes to a separate branch

* remove commented out line from editor file

* rename welcome screen event for clarity

* add Welcome CI Test to ciTests.ts update test
re-arrange buttons for initial positioning

* conver buffer manager test to jest

* add test for buffer layer input handlers

* add tests for welcome commands view and extra test to buffer manager test

* [WIP] render oni sessions on welcome page

* add sessions to welcome screen and redesign commands section of screen

* create separate components for left and right section
add icon to sessions list

* add [WIP] section navigation

* rename event args and handle lateral movement (minimally)

* add fallback for empty session, remove console.log in session store
fix accidentally triggered session restoration

* fix lint errors

* update tests for welcome layer changes

* fix lint errors

* render welcome even if no version, handle single section
add snapshot and update tests

* add set state and increase tests for welcome component

* update getMetadata function to use async await + fsExtra

* increase header font size, reduced box size
seperated selectedItem selected styles into css block
add margin to icon styles

* update snapshot
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants