Added delete-to-next/previous-word-boundary

[ghost]

This is better behavior for alt-del/backspace operations, is less "greedy" and doesn't eat up excess characters.
Also set default key mappings to these, feel free to trash my changes to keymaps/darwin.cson. Though I do feel this should be default behavior, personally.

[ghost]

Any update on this?

[dogancelik]

Can someone merge this?

[50Wliu]

@CaptSaltyJack could you please rebase this onto latest master? There's conflicts right now.

[ghost]

@50Wliu My git skills aren't mad enough, I'd probably wind up destroying my repo. Shouldn't I just do git merge master to merge updates into the feature branch?

[benogle]

git checkout master
git pull
git checkout my-branch
git rebase master

then fix all the conflicts

git add ... # all the files conflicted
git rebase --continue

Then when all the conflicts are fixed

git push --force

[ghost]

Done! git pull did nothing since that was my own remote (origin). I had to merge from upstream/master first.

[benogle]

Oh right, forks.

[ghost]

Just curious, why rebase over merge? Is it to eliminate all the history I've created, squashing all those commits into a single one?

[50Wliu]

Not too sure about rebase vs merge, but I've generally always used rebase when I had conflicting changes.

Also, could you please add your changes to win32.cson and linux.cson if/when the changes to darwin.cson get a 👍?

EDIT: .cson, not .json. Whoops.

[ghost]

Yeah, as I understand it, rebase wipes out a branch's history. Which I guess is OK when people are merging changes into a project like this. No one needs to see my 20 revisions saying "fixed that one thing again." All that matters is the single commit with one comment as it's merged into upstream/master.

[lee-dohm]

Is this supposed to be editor.deleteToPreviousWordBoundary()?

[ghost]

@lee-dohm Oops.. yes, probably. I don't have the test text handy so I can't verify, but it doesn't make any sense that the test doesn't even call the new function.

[benogle]

Rebasing does not create unnecessary merge commits. Basically its as if you just developed this on the latest version of master. Your history is still there, but the commit shas are different.

[lee-dohm]

I ran the specs from this PR with apm test on Atom change adb0178 on Mac OS X 10.10.3 and both "when no text is selected" tests are failing. Could you fix those, @CaptSaltyJack?

[ghost]

I could never test this properly because of console errors within Atom when running the (if I recall the name) Package Specs command. To clarify, I tested this thoroughly myself, but the test specs I was never able to verify. @benogle is familiar with the huge headache I went through to try to get the test specs working. I gave up. I apparently can't even run atom -d without errors.

Uncaught TypeError: Cannot read property 'dispatch' of /Users/bob/github/atom/src/space-pen-extension.coffee:64

[lee-dohm]

Ok, I'll take a look and fix the tests 😀

[ghost]

Even apm test bombs out:

[17216:0429/172638:INFO:CONSOLE(27)] "Error: Module version mismatch. Expected 43, got 41.

It would be cool to have an atom developer's guide somewhere that has clear steps on how to set up a dev environment. I could never get mine 100% functional.

[ghost]

Thanks @lee-dohm. I'm sure it's something simple/dumb.

[lee-dohm]

You're most welcome @CaptSaltyJack 😃

[benogle]

I am not comfortable changing the default keybinding. I say we merge this without the default bindings changed, and have some folks change their keybindings before changing the default.

[ghost]

@benogle That makes sense to me. Probably best not to change default behaviors after people are already used to what's been the default since the beginning.

[lee-dohm]

@CaptSaltyJack, I have a question about the third clause to ".deleteToNextWordBoundary() when no text is selected deletes to the next word boundary", starting on line 2384 which is:

          editor.deleteToNextWordBoundary()
          expect(buffer.lineForRow(0)).toBe 'var quicksort = () {'
          expect(buffer.lineForRow(1)).toBe '  var sort = function(it {'
          expect(cursor1.getBufferPosition()).toEqual [0, 15]
          expect(cursor2.getBufferPosition()).toEqual [1, 24]

I corrected the typos and mismatches (row numbers and cursor positions), but the question is that before this portion of the test is executed, the text for line 0 should be var quicksort =|function () { with the pipe character representing the position of the cursor. My understanding is that the intent of this PR is that it is not supposed to be greedy about taking whitespace too. Your test states that the result should be var quicksort =| () {, but the actual result is var quicksort =|() {. Is this a bug?

The same bug appears in the same test on line 2392.

[lee-dohm]

Pushing my updates to fix most of the tests.

[lee-dohm]

Looking to see if I can fix it.

[ghost]

Geez, was I drunk when I wrote this test? Pushing..

[ghost]

Unfortunately I can't even test my own code now, as I can't run Atom in dev mode. @benogle, what were the commands to reset the atom dev environment over again? Something about clean, bootstrap, build?

[lee-dohm]

I think you're talking about this command @CaptSaltyJack? apm/node_modules/atom-package-manager/bin/apm rebuild

Oh, no ... got ahead of myself ...

This is generally what I use when I want to reset everything ...

script/clean
script/build

[ghost]

@lee-dohm Maybe. I'm rebuilding now to see if it fixes atom -d so I can double-check the functionality of my changes.

[lee-dohm]

Merged as #6575. Thanks @CaptSaltyJack 👍