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

Buffer refactor #535

Open
wants to merge 44 commits into
base: develop
Choose a base branch
from

Conversation

JohnTheAppleSeed
Copy link
Contributor

This is a massive refactor, which brings with it several nifty features:

  • better support for CRLF (but we're not there yet);
  • speed (block operations are significantly faster);
  • a few new features, and proper support for D in visual mode
  • support for XCode 5.1
  • native undo support
  • better layering : XVimBuffer, XVimView and hopefully eventually better testability.

It has stability improvements all over the board, fixes for end-of-line motions, ...

This object will now be the main entry point to manipulate the content of
the textStorage, so that we don't have to extend the NSTextStorage with
categories.

XVimBuffer is now created each time we learn about a new Document that
responds to -textStorage which XCode documents do.

XVimBuffer also knows about the undo manager, and is used to be the target
of the specific vim action we mean to undo, which means that undoing the
cursor positions won't crash anymore (Layout managers and textviews come
and go, and it's not correct to make them the target of an undo
operation).
XVimTextStoring becomes a protocol that you implement when you intend to
give better alternatives than the default implementations from XVimBuffer.

This commit doesn't alter code, it merely moves some around, and adpt
callers.
This makes editing the text-storage faster than going through the view
obviously.

Use this as a proof of concept to reimplement the bulk of xvim_shift
as -[XVimBuffer shiftLines:column:count:right:block:]

The performance that this code reaches now is bluffing compared to the
previous one. It's still not nearly as fast as vim, but it's decent.

Note that the cursor position handling code is still messy, which makes
the number of parameters to that selector properly disgusting, but we're
getting there.
Move them to a proper ivar of the XVimBuffer instead
This is the beginning of moving a lot of stuff there, so that we can
capture the buffer state on undo/redo properly.
As a side effect, this means we now support CRLF properly for this
command, which should help for XVimProject#530

Fix a couple of issues in the XVimBuffer methods that this new code
spotted.
Hooker is really a disgusting hack which uses Hook classes for no good
purposes and does no compile time check.

Let's use this to modify the behavior of (typically) NSTextView and
NSTextStorage if they have xvim_* objects associated with them.
+[XVimView initialize] swizzles NSTextView like we used to do on the DVT
one. Nothing was really DVT speciic in the DVTSourceTextViewHook in the
first place. The swizzling does nothing if there is no XVimView associated
with the NSTextView, or if XVim is disabled.  All that is non specific to
XCode.

Rewrite IDEEditorHook as an IDEEditor+XVim category with swizzling.
In the -didSetupEditor override, detect when there are
DVTSourceTextView's that are the one we know what to do with.

Check their document has a file:// URL (so that the script editor in the
Xcode rules are ignored). If the textStorage happens to already be
created, allocate the XVimBuffer right away.

In theory we would have to check when the document for a given TextView
changes whether we still support XVim, but XCode actually destroys and
recreates the view each time you switch documents, so it doesn't matter.
It's disabled by default because it only make sense for debugging
purposes.
And fix a silly bug with the swizzling category when the class doesn't
override the selector we're swizzling.
it's replaced by XVimView/XVimBuffer.
It's a nice reference of the things to have though.
Do not hardcode XCode path to /Applications/Xcode 2.app, and find the
framework relative the $DEVELOPER_DIR, which will do what's right.

Upgrade warnings for XCode 5
It's been a huge help for development for me.
Fold the Tilde/Lower/Upper (and Rot13) into a single SwapChar evaluator.
Fix various repeat issues with those.

Use it in the normal, visual, GAction and GVisual evaluators as required.

This obviously uses the new undo architecture, hence should be faster
This is a huge refactor that makes the code more independant, more
efficient, and acting on XVimView's and XVimBuffer's as much as possible.

In particular, this commit tries not to use any NSTextView methods to
modify the content, which has several important side effects:
- it's more efficient, since we don't go through all the XCode logic to
  handle edits (it's really a waste when we're doing block operationst
  typically), performance for several operations is visibly faster than it
  used to be;
- it's better for the user as this goes below XCode AutoCompletion code,
  and doesn't pop up spurious dialogs when you're in command mode anymore;
- this goes through our Undo logic which allows exact coalescing and
  cursor placement, instead of relying on the Appkit doing the right thing
  before. (and it doesn't for insertion, but we'll get to that at some
  point).

This commit also does tons of cleanups with the selection and movement,
notably fixing behavior when MOTION_END_OF_LINE ($) is used, as this
should be preserved, and wasn't

Note that for now XVimView is still XCode dependant, because I didn't
write a category to swizzle XCode classes, and didn't write a protocol
for the TextView yet. Such a protocol will include anything that is
rendering related, like placeholder handling. Indenting should go to the
NSTextStorage instead, as it's strictly content related.

I wouldn't be surprised there are a couple of regression, crashers, but
the test-suite passes, and I'm living on that patch now.
I had a bug because it's not on by default in XVim and a property was
automatically synthesized because I didn't implement the selector and had
no warning.
And implement all the forwarders all the way down.
Hide some -is* from the NSTextStorage category, move isLastLine as
isIndexOnLastLine on XVimBuffer.
btw it's not 100% correct because gv doesn't quite do the right thing yet.
Those functions are related to position of the index within the line.
This is now:
    -indexOfCharMotion:index:options:
    -indexOfLineMotion:index:column:

Implement _/+/-/$/CR/C_m/... using this.

Also rename and properly namespace MOTION_OPTION as XVimMotionOptions and
the MOPT_ prefix.
When normal-mode replace failed, it used to fail to quit insert mode,
which broke several motions. Make -escapeFromInsert take a boolean to know
whether in addition to quitting INSERT mode, we should also go backwards.

Replace also had an off-by-one that prevented to replace the Last char of
the line (a >= comparison really should be >).

Add a BAR selector to the XVimMotionEvaluator, and a new
MOTION_COLUMN_OF_LINE enum value.
The reason for that is that when undo is generated by XCode it can leave
us in really awkward places.

Example, go to the end of a line, type:

C<ESC>
i.<ESC>
u
<-- at this point, your cursor is on the end-of-line where it shouldn't be
since we're in normal mode.

When the undo are generated by us, this of course never happens (or would
be a bug).
Use the line endings we guess from the first line of the file.
This, really fixes dd for me on CRLF files now.

It also means we support Classic Mac line endings too (CR-ended lines)

Also compute the end-of-line to eat when deleting/yanking at the end
properly.

This should make things better for XVimProject#530
Reduce the number of calls on each keystroke by avoiding the
-selectorForInstance + instanceResponds + ... calls

Avoid using NSStrings to compute the selector, it does malloc()s, a simple
buffer + sel_getUid() is faster

Use the NS*FunctionKey values from Foundation instead of hardcoded values.
And now my XVim stops assert()ing when I use F13 by mistake.

Avoid using self.{modifier,character} but access the ivars directly, it's
faster.

Use iswprint() instead of isPrintable, it's supposed to do what's right...
both sound like a very bad idea, move that stuff to the XVimView where it
belongs. The less we swizzle, the better we are.
@xverges
Copy link

xverges commented Jan 21, 2014

I've been using this branch for a few days with no problem. Today, switching a tab, Xcode died and blamed XVim. I was switching from a tab with a comparison view to a tab with a regular editor. I did the same operation after restarting Xcode and everything worked. I'm posting it just in case it helps to spot something wrong, but this doesn't look like something that I would be able to recreate.

Process:         Xcode [18769]
Path:            /Applications/Xcode.app/Contents/MacOS/Xcode
Identifier:      com.apple.dt.Xcode
Version:         5.0.2 (3335.32)
Build Info:      IDEApplication-3335032000000000~4
App Item ID:     497799835
App External ID: 15588541
Code Type:       X86-64 (Native)
Parent Process:  launchd [137]
User ID:         501

PlugIn Path:       /Users/USER/Library/Application Support/Developer/*/XVim
PlugIn Identifier: net.JugglerShu.XVim
PlugIn Version:    1.01 (1)

Date/Time:       2014-01-21 09:02:47.682 +0100
OS Version:      Mac OS X 10.8.5 (12F45)
Report Version:  10

Interval Since Last Report:          595289 sec
Crashes Since Last Report:           1
Per-App Interval Since Last Report:  435252 sec
Per-App Crashes Since Last Report:   1
Anonymous UUID:                      02B619E2-11A7-B092-C06F-30F50E0604AE

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000

Application Specific Information:
Sending _didClickTabButton: to <DVTTabBarView: 0x7faacba65ef0> from <DVTTabButton 0x7faacbf12b60 "xxxxxxx_xxxxxxxxxxxxxxxxx.h">
ProductBuildVersion: 5A3005
ASSERTION FAILURE in /SourceCache/DVTKit/DVTKit-3553/Framework/Classes/TextCompletion/DVTCompletingTextView.m:1335
Details:  Attempt to select an invalid range of text: {18446744073709551615, 0}. Text length: 8996. (Please file a Radar. OK to Continue from here.)
Object:   <DVTSourceTextView: 0x7faace307710>
Method:   -setSelectedRange:
Thread:   <NSThread: 0x7faac9414be0>{name = (null), num = 1}
Hints:   
  0: Sending _didClickTabButton: to <DVTTabBarView: 0x7faacba65ef0> from <DVTTabButton 0x7faacbf12b60 "xxxxxxx_xxxxxxxxxxxxxxxxx.h">
Backtrace:
  0  0x0000000108758ea8 -[IDEAssertionHandler handleFailureInMethod:object:fileName:lineNumber:messageFormat:arguments:] (in IDEKit)
  1  0x00000001074df7a5 _DVTAssertionHandler (in DVTFoundation)
  2  0x00000001074dfad4 _DVTAssertionFailureHandler (in DVTFoundation)
  3  0x0000000107a081df -[DVTCompletingTextView setSelectedRange:] (in DVTKit)
  4  0x0000000107a316a4 -[DVTSourceTextView setSelectedRange:] (in DVTKit)
  5  0x00007fff87daca20 _NSSetRangeValueAndNotify (in Foundation)
  6  0x0000000112954343 -[XVimView adjustCursorPosition] at /Users/xavier/tools/XVimFix/XVim/XVimView.m:706 (in XVim)
  7  0x0000000112922823 -[XVimWindow syncEvaluatorStack] at /Users/xavier/tools/XVimFix/XVim/XVimWindow.m:341 (in XVim)
  8  0x00000001129215ea -[XVimWindow _bufferChangedNotification:] at /Users/xavier/tools/XVimFix/XVim/XVimWindow.m:168 (in XVim)
  9  0x00007fff8f2f4eda _CFXNotificationPost (in CoreFoundation)
 10  0x00007fff87ccb7b6 -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation)
 11  0x00000001128ebe4e -[XVim observeValueForKeyPath:ofObject:change:context:] at /Users/xavier/tools/XVimFix/XVim/XVim.m:242 (in XVim)
 12  0x00007fff87d0e7b7 NSKeyValueNotifyObserver (in Foundation)
 13  0x00007fff87d0603a -[NSObject(NSKeyValueObserverRegistration) _addObserver:forProperty:options:context:] (in Foundation)
 14  0x00007fff87d04e44 -[NSObject(NSKeyValueObserverRegistration) addObserver:forKeyPath:options:context:] (in Foundation)
 15  0x0000000112935901 -[IDEEditor(XVim) xvim_didSetupEditor] at /Users/xavier/tools/XVimFix/XVim/IDEEditor+XVim.m:99 (in XVim)
 16  0x000000010b08dd78 -[IDESourceCodeEditor didSetupEditor] (in IDESourceEditor)
 17  0x000000010872b25d -[IDEEditorContext _notifyDelegateAndOpenNavigableItem:withContentsURL:documentExtensionIdentifier:locationToSelect:annotationRepresentedObject:stateDictionary:annotationWantsIndicatorAnimation:exploreAnnotationRepresentedObject:highlightSelection:skipSubDocumentNavigationUnlessEditorIsReplaced:] (in IDEKit)
 18  0x000000010872c501 -[IDEEditorContext _notifyDelegateAndOpenEditorHistoryItem:previousHistoryItemOrNil:skipSubDocumentNavigationUnlessEditorIsReplaced:] (in IDEKit)
 19  0x0000000108728190 -[IDEEditorContext _openEditorHistoryItem:previousHistoryItemOrNil:skipSubDocumentNavigationUnlessEditorIsReplaced:] (in IDEKit)
 20  0x00000001085ca7f8 -[IDEEditorContext _openEditorHistoryItem:updateHistory:] (in IDEKit)
 21  0x00000001085c9a84 -[IDEEditorContext _openEditorHistoryItemFromStateSaving:] (in IDEKit)
 22  0x00000001085fec2b -[IDEEditorBasicMode _setPersistentRepresentation:forIdentifier:] (in IDEKit)
 23  0x000000010891d2a4 __60-[IDEEditorModeViewController _setPersistentRepresentation:]_block_invoke (in IDEKit)
 24  0x00007fff8f33b4a6 __NSArrayEnumerate (in CoreFoundation)
 25  0x00000001085c58d3 -[IDEEditorModeViewController _setPersistentRepresentation:] (in IDEKit)
 26  0x00000001085c56d3 -[IDEEditorModeViewController revertStateWithDictionary:] (in IDEKit)
 27  0x0000000107486775 -[DVTStateToken _pullStateFromDictionary:] (in DVTFoundation)
 28  0x0000000107486596 -[DVTStateToken pullStateFromRepository] (in DVTFoundation)
 29  0x00000001085a6e02 -[IDEViewController revertState] (in IDEKit)
 30  0x0000000108581fbb -[IDEEditorArea _updateStateSavingRegistrations] (in IDEKit)
 31  0x00000001085a9229 -[IDEEditorArea _refreshEditorContextsAndPreserveCurrentEditorHistoryStack:] (in IDEKit)
 32  0x0000000108751728 __31-[IDEEditorArea viewDidInstall]_block_invoke (in IDEKit)
 33  0x00000001074f1618 -[NSObject(DVTObservingConvenience) _dvt_newObserverForKeyPath:options:owner:withHandlerBlock:] (in DVTFoundation)
 34  0x00000001074715d3 -[NSObject(DVTObservingConvenience) dvt_newObserverForKeyPath:options:withHandlerBlock:] (in DVTFoundation)
 35  0x00000001085a3616 -[IDEEditorArea viewDidInstall] (in IDEKit)
 36  0x0000000112935d1f -[IDEEditorArea(XVim) xvim_viewDidInstall] at /Users/xavier/tools/XVimFix/XVim/IDEEditorArea+XVim.m:61 (in XVim)
 37  0x0000000107aeb08f -[DVTViewController _didInstallContentView:] (in DVTKit)
 38  0x0000000107a0e97e -[DVTControllerContentView _viewDidInstall] (in DVTKit)
 39  0x0000000107a0e222 -[DVTControllerContentView viewDidMoveToWindow] (in DVTKit)
 40  0x00007fff8856a2e7 -[NSView _setWindow:] (in AppKit)
 41  0x00007fff8f33b4a6 __NSArrayEnumerate (in CoreFoundation)
 42  0x00007fff8856a2c4 -[NSView _setWindow:] (in AppKit)
 43  0x00007fff8f33b4a6 __NSArrayEnumerate (in CoreFoundation)
 44  0x00007fff8856a2c4 -[NSView _setWindow:] (in AppKit)
 45  0x00007fff8f33b4a6 __NSArrayEnumerate (in CoreFoundation)
 46  0x00007fff8856a2c4 -[NSView _setWindow:] (in AppKit)
 47  0x00007fff88573a77 -[NSView addSubview:] (in AppKit)
 48  0x00007fff885845af -[NSView replaceSubview:with:] (in AppKit)
 49  0x00007fff886c1464 -[NSTabView _switchTabViewItem:oldView:withTabViewItem:newView:initialFirstResponder:lastKeyView:] (in AppKit)
 50  0x00007fff886c0d14 -[NSTabView selectTabViewItem:] (in AppKit)
 51  0x0000000107ab4347 -[DVTTabSwitcher selectTabViewItem:] (in DVTKit)
 52  0x00000001085aa424 -[IDEWorkspaceWindowController _showTab:] (in IDEKit)
 53  0x0000000108626ebc -[IDEWorkspaceWindowController selectTab:] (in IDEKit)
 54  0x0000000107ab60a3 -[DVTTabBarView _didClickTabButton:] (in DVTKit)
 55  0x00007fff88690959 -[NSApplication sendAction:to:from:] (in AppKit)
 56  0x0000000107bc1aff __37-[DVTApplication sendAction:to:from:]_block_invoke (in DVTKit)
 57  0x00000001074e0ab1 DVTInvokeWithFailureHint (in DVTFoundation)
 58  0x00000001079f35ba -[DVTApplication sendAction:to:from:] (in DVTKit)
 59  0x00007fff886907b7 -[NSControl sendAction:to:] (in AppKit)
 60  0x0000000107ab5c2a -[DVTTabButton _considerDragFromMouseDown:] (in DVTKit)
 61  0x0000000107ab5a6b -[DVTTabButton mouseDown:] (in DVTKit)
 62  0x00007fff8868550e -[NSWindow sendEvent:] (in AppKit)
 63  0x00007fff88681644 -[NSApplication sendEvent:] (in AppKit)
 64  0x000000010855b83b -[IDEApplication sendEvent:] (in IDEKit)
 65  0x00007fff8859721a -[NSApplication run] (in AppKit)
 66  0x00007fff8853bbd6 NSApplicationMain (in AppKit)
 67  0x00007fff882887e1 start (in libdyld.dylib)

abort() called

@JugglerShu JugglerShu force-pushed the develop branch 2 times, most recently from dd0deb9 to 125c207 Compare September 20, 2015 17:17
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.

2 participants