[CLOSED] Overwrite mode improvements

[core-ai-bot]

Issue by WebsiteDeveloper
Thursday Feb 06, 2014 at 08:55 GMT
Originally opened as adobe/brackets#6777

---

Combined adobe/brackets#6670 and adobe/brackets#6328 into this pull request as requested.
Waiting for review.
@redmunds

---

WebsiteDeveloper included the following code: https://github.com/adobe/brackets/pull/6777/commits

[core-ai-bot]

Comment by WebsiteDeveloper
Thursday Feb 06, 2014 at 21:41 GMT

---

@redmunds fixes pushed.

[core-ai-bot]

Comment by redmunds
Thursday Feb 06, 2014 at 22:02 GMT

---

Done with initial review. This works great!

Only other issue is that the cursor should change to a hand (pointer) when mouse is over status bar.

Once all issues have been addressed, this branch should be rebased. After I merge it, then I'll open bug against CodeMirror for scrollCursorIntoView() issue.

[core-ai-bot]

Comment by redmunds
Tuesday Feb 11, 2014 at 23:04 GMT

---

@WebsiteDeveloper Maybe you missed my "Done with review" comment, so tagging you on it this time.

[core-ai-bot]

Comment by WebsiteDeveloper
Wednesday Feb 12, 2014 at 10:13 GMT

---

@redmunds i absolutely missed that.
I rebased the branch and fixed the two minor issues you noted

[core-ai-bot]

Comment by redmunds
Wednesday Feb 12, 2014 at 16:03 GMT

---

@WebsiteDeveloper Done with review. This is looking great. 1 comment about whitespace "fixing".

Could you also add a couple tests to the EditOptionHandler-test tests? I think you could use SpecRunnerUtils.simulateKeyEvent() to test that doc is correctly updated in both insert and overwrite modes. This would be ok as a separate pull request.

[core-ai-bot]

Comment by WebsiteDeveloper
Wednesday Feb 12, 2014 at 16:12 GMT

---

@redmunds
Alright i will add a couple of tests i a subsequent pull.
As to the whitespaces is it sufficient if i add the one newline at the end and leave the rest as they are?

[core-ai-bot]

Comment by redmunds
Wednesday Feb 12, 2014 at 16:21 GMT

---

@WebsiteDeveloper Are you using an extension that automatically updates the whitespace? If so, Brackets (as of Sprint 36) now supports project-level preferences so can you disable (or change params for) that extension in the Brackets project?

[core-ai-bot]

Comment by WebsiteDeveloper
Wednesday Feb 12, 2014 at 16:26 GMT

---

I used to. But i removed it.

[core-ai-bot]

Comment by redmunds
Wednesday Feb 12, 2014 at 16:31 GMT

---

@WebsiteDeveloper

is it sufficient if i add the one newline at the end and leave the rest as they are?

I can live with it one last time :)

[core-ai-bot]

Comment by WebsiteDeveloper
Wednesday Feb 12, 2014 at 20:57 GMT

---

alright

[core-ai-bot]

Comment by WebsiteDeveloper
Wednesday Feb 12, 2014 at 21:01 GMT

---

@redmunds added the newline

[core-ai-bot]

Comment by redmunds
Wednesday Feb 12, 2014 at 22:05 GMT

---

Looks great. Thanks. Merging.