Implement Patch in C++ #12

maxbrunsfeld · 2016-11-02T00:43:17Z

Methods needed for the new DisplayLayer refactor

splice(start, deletedExtent, insertedExtent)
spliceOld(start, deletedExtent, insertedExtent)
getHunks()
getHunkInOldRange(oldStart, oldEnd)
getHunkInNewRange(newStart, newEnd)
hunkForOldPosition(oldStart)
hunkForNewPosition(oldStart)

Other methods needed to for existing use cases

Signed-off-by: Nathan Sobo <[email protected]>

Signed-off-by: Damien Guard <[email protected]>

Signed-off-by: Max Brunsfeld <[email protected]>

@maxbrunsfeld

When creating a brand new buffer by round-tripping the data through a base-64 encoded string, the existing approach wasn't accessing the data correctly and the serialization version check was failing comparing against what looked like garbage data. I tracked down this API which seems simpler and works with buffers that have been created from base-64-encoded strings. /cc @maxbrunsfeld

nathansobo · 2016-12-09T23:17:43Z

@maxbrunsfeld This should be good to merge as soon as we figure out these linker errors on Travis 😿.

nathansobo · 2016-12-09T23:23:49Z

Trying to run the JS tests only to see if this is related to the configuration of the native tests.

nathansobo · 2016-12-09T23:26:33Z

Okay, looks like it's the configuration of the test target on Linux.

nathansobo · 2016-12-09T23:45:45Z

I wonder if this has something to do with trying to link the tests executable against V8 since we include nan.h in patch.cc. We do that just so we can use Nan::Maybe.

For some reason, building the tests in debug mode on Linux causes linker issues that I don't want to solve.

nathansobo · 2016-12-10T03:15:56Z

Building the tests without the --debug flag works on Linux. Not sure why --debug causes linker errors.

By specifying the -Og compiler flag we get "debugger-friendly" optimizations. This removes references to symbols in nan.h that are only available in the Node binary and that can't be resolved when linking our tests executable.

nathansobo · 2016-12-10T04:24:11Z

Okay, I figured it out. I think when we compiled the tests executable with GCC, the default optimization level of -O0 was causing a bunch of V8 symbol references from nan.h to be retained in the object files. When building a shared library this doesn't matter because they can be resolved at load time, but when building an executable these symbols need to be resolved eagerly and since we don't actually have Node as a library, the linker fails.

Replacing -O0 with -Og in the Debug configuration on Linux applies "debugger-friendly" optimizations and seems to do enough eliminate the undefined references in nan.h since we don't use them in patch.cc. The docs claim that -Og won't apply any aggressive optimizations that could interfere with debugging.

maxbrunsfeld · 2016-12-10T08:04:12Z

Yeah, so HunkForPosition is the only method that depends on v8's Maybe type, and we don't call this method in our tests, so if we enable any optimizations at all, the method will be removed from the executable, eliminating the v8 references.

In light of atom-archive/buffer-offset-index#2, we might want to just remove the v8 dependency from our 'core' Patch code, and only use it in binding.cc. We could just represent the maybe value as a std::pair<Hunk, bool> or something.

nathansobo · 2016-12-10T19:02:28Z

There's a boost header-only library that implements optionals that I think is going to be standardized in C++17.

maxbrunsfeld added 17 commits October 31, 2016 20:05

Start work on C++ Patch

1bbe2e2

Signed-off-by: Nathan Sobo <[email protected]>

Add Patch destructor

5fba88d

Add native test for non-overlapping splices

95be575

Add missing include path in clang_complete file

b677fbf

Run mocha on on JS files in npm test

223bac4

Add Patch.printDotGraph method, for debugging

669a5a4

Make JS hunk properties camel case

d9d1e2d

Fix bugs in native Patch.splice

33aebbc

Signed-off-by: Nathan Sobo <[email protected]>

Add Patch.getHunksInNewRange

9d3061c

Signed-off-by: Nathan Sobo <[email protected]>

Add test-with-debug-graph script

2d30e56

Add Patch.getHunksInOldRange

a75e505

Add Patch.translateOldPosition and .translateNewPosition

1ec9d5a

Use NVM_PATH env var in .clang_complete file

ce4f907

Add segfault-handler as dev dependency

9886c95

Don't keep .clang_complete file in version control

0f462e1

Make Patch helper functions instance methods

3aca0d6

Add Patch serialization and deserialization methods

302bf1a

Signed-off-by: Damien Guard <[email protected]>

maxbrunsfeld force-pushed the native branch from c9f8ce0 to 302bf1a Compare November 3, 2016 19:55

maxbrunsfeld and others added 6 commits November 3, 2016 13:27

Clean up patch serialization code

0420180

Fix compiler errors on travis

6193600

Use new gcc on travis

17451c6

Add clip mode parameter to position translation

dbca25a

Signed-off-by: Max Brunsfeld <[email protected]>

Default clipMode to CLOSEST

bce0896

Replace translate{Old,New}Position with hunkFor{Old,New}Position

4c4bdc0

maxbrunsfeld force-pushed the native branch from ea8cc0e to 4c4bdc0 Compare November 4, 2016 17:36

maxbrunsfeld added 2 commits November 7, 2016 10:01

Add appveyor.yml

bc01c99

Always serialize numbers in network byte order

8665bb9

maxbrunsfeld force-pushed the native branch from bcc0ae4 to 8665bb9 Compare November 7, 2016 18:16

maxbrunsfeld added 2 commits November 7, 2016 10:41

Link ws2_32 on windows

5fd6a50

Don't run native tests on windows

f73919e

Nathan Sobo added 6 commits December 8, 2016 17:36

Add Patch.invert

4f31998

Test Patch.compose argument checking

fcd57cc

Add non-enumerable legacy properties for backward compatibility

96ad5cb

Fix invert call in test

24fbd3e

Remove electron-mocha dependencies

7f0a446

nathansobo force-pushed the native branch from 2b592b4 to 155eb03 Compare December 9, 2016 22:40

Remove JS implementation

8a5a53a

nathansobo force-pushed the native branch from 155eb03 to 8a5a53a Compare December 9, 2016 22:46

Try just running test-js on Travis

2c709fc

nathansobo force-pushed the native branch 3 times, most recently from e1640ff to 7fd6357 Compare December 9, 2016 23:37

Don't build in debug mode on Travis

dfb5fcb

For some reason, building the tests in debug mode on Linux causes linker issues that I don't want to solve.

nathansobo force-pushed the native branch from 7fd6357 to dfb5fcb Compare December 10, 2016 03:11

Nathan Sobo added 4 commits December 9, 2016 20:45

Avoid linker errors for native tests on Linux

e3b4aa5

By specifying the -Og compiler flag we get "debugger-friendly" optimizations. This removes references to symbols in nan.h that are only available in the Node binary and that can't be resolved when linking our tests executable.

🎨

1c7e067

Require build/Debug/atom_patch if present, else use build/Release

65a47e3

Only override -O flag for Debug configuration on Linux

a0abb61

nathansobo merged commit 955a794 into master Dec 10, 2016

nathansobo deleted the native branch December 10, 2016 04:24

maxbrunsfeld mentioned this pull request Dec 14, 2016

Optimize DisplayLayer's memory usage by storing transformations in a Patch atom/text-buffer#185

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement Patch in C++ #12

Implement Patch in C++ #12

Uh oh!

maxbrunsfeld commented Nov 2, 2016 •

edited by nathansobo

Loading

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 10, 2016

Uh oh!

nathansobo commented Dec 10, 2016 •

edited

Loading

Uh oh!

maxbrunsfeld commented Dec 10, 2016 •

edited

Loading

Uh oh!

nathansobo commented Dec 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement Patch in C++ #12

Implement Patch in C++ #12

Uh oh!

Conversation

maxbrunsfeld commented Nov 2, 2016 • edited by nathansobo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Methods needed for the new DisplayLayer refactor

Other methods needed to for existing use cases

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 9, 2016

Uh oh!

nathansobo commented Dec 10, 2016

Uh oh!

nathansobo commented Dec 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxbrunsfeld commented Dec 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nathansobo commented Dec 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maxbrunsfeld commented Nov 2, 2016 •

edited by nathansobo

Loading

nathansobo commented Dec 10, 2016 •

edited

Loading

maxbrunsfeld commented Dec 10, 2016 •

edited

Loading