EpicEditor TAB implementation

Issue-159.md

“Smart tabs” sound better, a quick look at configuration files for both vim and sublime text:

Vim 
---
noexpandtab
tabstop
smartindent
smarttab

Sublime Text 
------------
tab_size
translate_tabs_to_spaces
use_tab_stops
...

smarttab in vim is used to respect <BS> character in some circumstances. For example if a tab key is to insert 4 spaces, a <BS> will delete 4 spaces if it is performed after smartindent/expandtab inserts 4 spaces in front of a line.

In the case of EpicEditor, we are combining some of these features into one option:

• Enable tab key (and shift-tab);
• Don't expand tab to spaces (and use css: tab-size instead) – I think expansion of tabs could be useful, but may introduce too many options/variables. But it is something we can discuss: my position is to keep it simple, at least for this first implementation. But I will experiment with this, keen to see how this will work;
• By enabling tab key, we introduce smart indentation (rule where previous lines' indentation is respected).

What I can do is try and experiment with internal options such as smartIndentation, smartTabs, expandTab and see how they pull up together. I am quite keen on seeing how others implement this first.

We can always change the name later on. I'll keep it as smartTabs for now.

Oh! Great idea on that CSS trick. Just add that to the editor CSS in in the load method somewhere.

textContent sucks. I tried using it. Use innerText. Only Firefox doesn't support it and it's easier to shim innerText than textContent. Also, it'll be less work since IE5.5?, Chrome, and everyone else supports innerText.

Look how easy it is in _getText and look at the nasty shim for Firefox.

Just thinking out loud here on what needs to be done:

1. Handling of enter keys so browser specific tags are omitted (already started here feature/newline – this is based on dabblet, currently refactoring to satisfy EpicEditor)
2. Optional handling of paste command to strip out dirty html tags (feature/paste)
3. Indentation (including outdentation) (feature/indentation)
4. Remember indentation level (same branch as above)

If I nail 1 and 2, the rest should be fairly easy and straight forward!

#1 Would fix the inconsistent \r\n in Windows / IE and just \n on Windows and Mac every other browser. Now in IE you have to hit shift enter to get a single \n. It's not a big deal, but would be nice if it was the same.

Progress report, TLDR; TAB will have to wait until we get this \n sorted. The pure text version will make a lot of things easier! (I think).

Preliminary newline (\n) support is in feature/newline

Cannot test in IE7/8 atm. It looks like the develop branch isn't working on IE7/IE8 at this stage. I have tested my feature branch against IE9, Safari 5.2, Safari 6, Firefox 13, Chrome 20. There is a known issue on Opera 12 – it just doesn't work.

To do for this feature (listed in order):

1. Merge between develop and feature/newline is not pretty. Even if I rebase, I will need tinker with my code. But it needs to be done;
2. Write tests so future modifications don't break;
3. Refactor text insertion code inside newline function so that issue #162 can be addressed;
4. Change textContent to innerText;
5. Make sure new tests pass in IE8+ and Opera 12 (incl. other browsers mentioned above).

Proposed workflow for getting TAB in to main branch:

1. Finish feature/newline
2. Send pull request
3. Start feature/indentation and feature/paste and it will reuse some of the new API methods introduced in feature/newline

This will keep things branch/merging quite linear and pretty.

Yep, IE8 doesn't work yet. It's on my 0.1.2 to do list but it's been a pain. I didn't try to get it compatible with IE8 from the start so now I'm having to go back and fix a bunch of shit that could have been a lot easier if I would have done it right at the beginning. That's why from now on I'm trying to be more strict about IE8 compatible code. IE7, maybe... IE8 for sure. It will go out for the next release tho, or I plan to. Also, don't worry too much about Opera. The editor is already borked in Opera anyway.

Merging

I did a quick merge and the merge conflicts look super easy to fix. Just take whatever is in develop. Doesn't look like any code you changed is conflicting with code I changed.

Code Review

As for a quick code review, looks great. We try to stick with single var statements, but other than that it all looks consistent and very clean! Nice work! Linting should actually fail with multiple vars, but I think @johnmdonahue changed that for debugging and just never changed it back, John? I think I'll add it back to the JSHint config.

API Questions / Discussion

Your new API methods like newline, getSelectionRange, etc, are those all meant to be public methods? Like, would a non-core dev ever need those? If you think any are just for internals don't forget to put a _ at the start of the method name otherwise we'll need to document and support them and have a reason why non-internal development would benefit from those methods.

How do you feel about getSelectionEnd/Start to match the other API names which are getter and setter patterns? Something else to think about is, in the rest of the API we have get{Something}("string") like getElement('editor') or getFile('foo'). Would it make sense to stay in that same style and do like getSelection(), getSelection('start'), getSelection('end') or something similar? Thoughts or concerns?

Here's some ways to write this out

// One (current):
var selEnd = editor.selectionEnd();

// Two (half match current style):
var selEnd = editor.getSelectionEnd();

// Three (match current style):
var selEnd = editor.getSelection('end');

// Four (totally different):
var selEnd = editor.getSelection().end;

Again, super exciting and great work! Can't wait to see this land!

We try to stick with single var statements

Yep, will change I have changed these on my branch.

Your new API methods like newline, getSelectionRange, etc, are those all meant to be public methods?

Absolutely not, not all of them anyway! Yep, I've been experimenting with public API calls like insertText(text) – which inserts given text at the current cursor/selection. I will definitely clean them up. Most of the methods should be private.

How do you feel about getSelectionEnd/Start to match the other API names which are getter and setter patterns?

At this stage I am thinking of making getSelection public, but selectionStart/selectionEnd into private methods. I will also change their names to _getSelectionStart and _getSelectionEnd.

The reason why getSelection should be public is because, EE's editor (EEE?) is inside an iframe element. Normally we'd use window.getSelection, but for EE we need to make sure we select the iframeDocument. This way, people can use getSelection just like they would on a normal HTML development environment.

---

I've been working on bits and pieces here and there, between home/work so please excuse the delay! I hope to get a solid chunk of work done this Sunday! Hopefully, I can ask you for a proper code review by then!

I like insertText. And totally makes sense about getSelection. Having tried working with it working with selections is hard and IE vs other browsers handle each differently. A standardized set of tools to handle selections for developers using the API would be awesome.

No rush! Do what you can when you want :)