[CLOSED] duplicateText amended to handle end-of-file scenarios, adding new line; ...

[core-ai-bot]

Issue by glortho
Wednesday Jun 27, 2012 at 18:19 GMT
Originally opened as adobe/brackets#1179

---

...testing thereof; Replacement for previous pull request. issue #926

---

glortho included the following code: https://github.com/adobe/brackets/pull/1179/commits

[core-ai-bot]

Comment by peterflynn
Thursday Jun 28, 2012 at 17:13 GMT

---

I'll review this in the next 1-2 days.

[core-ai-bot]

Comment by pthiess
Friday Jun 29, 2012 at 03:53 GMT

---

@jedverity: Before we can accept this into the project, we need you to sign the Contributor License Agreement -- http://dev.brackets.io/brackets-contributor-license-agreement.html

[core-ai-bot]

Comment by glortho
Friday Jun 29, 2012 at 11:57 GMT

---

I did sign that a couple of days ago. Do you not have a record of it? Let me know and I'll sign it again.

On Thursday, June 28, 2012 at 11:53 PM, Peter Thiess wrote:

@jedverity: Before we can accept this into the project, we need you to sign the Contributor License Agreement -- http://dev.brackets.io/brackets-contributor-license-agreement.html

---

Reply to this email directly or view it on GitHub:
adobe/brackets#1179 (comment)

[core-ai-bot]

Comment by peterflynn
Monday Jul 09, 2012 at 14:25 GMT

---

Great job@jedverity -- thanks for the solid fix! Apologies for the long delay in reviewing -- vacation got the better of me. Feel free to ping me on IRC if you want to discuss any of the comments (or just post back here of course).

I've verified your 'Contributor License Agreement' record, so no worries there.

[core-ai-bot]

Comment by glortho
Tuesday Jul 10, 2012 at 12:59 GMT

---

Thank you,@peterflynn ! I've pushed the changes as you requested (though not sure why it's showing up as my alternative username).

[core-ai-bot]

Comment by peterflynn
Thursday Jul 12, 2012 at 21:00 GMT

---

Two more changes needed and then I think we're good to merge! Thanks again!

Re "totalLines()" vs. lineCount(): yep, you're right -- my bad.

Re declaring all vars at the top: wouldn't be the first time it's been debated here :-) Officially the Brackets coding guide is neutral (either style is ok), but personally I have a pretty strong preference for declaring vars where they're initialized -- only in a project like Brackets where passing JSLint is a strict requirement. Combining JSLint with lower-down var declarations automatically catches the class of bugs where you use a var before it's been initialized (which JSLint is not smart enough to do if you move the var declaration earlier). The more bugs you find right when you hit Cmd+S, the less time spent wading around the debugger or the GitHub bug list later ;-)

[core-ai-bot]

Comment by peterflynn
Friday Jul 13, 2012 at 23:21 GMT

---

Great -- thanks! Merging now...