Closed Bug 474055 Opened 11 years ago Closed 11 years ago

Tabs are not supported correctly!

Categories

(Skywriter Graveyard :: Editor, defect, P2, critical)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dion, Assigned: GPHemsley)

References

Details

(Keywords: student-project)

Attachments

(2 files, 3 obsolete files)

Our document model doesn't support tabs right now so all hell breaks loose :)
Priority: -- → P1
Whiteboard: editor
Priority: P1 → P2
Group: mozilla-corporation-confidential
If this is about pressing TAB and it losing focus, I submitted a hacky patch to bug id #478386.  But if this is about the ASCII character 9, then ganbatte.
Duplicate of this bug: 474949
Assignee: nobody → gphemsley
OS: Mac OS X → All
Hardware: x86 → All
This is where I currently am in my progress towards supporting real tabs. This is a very preliminary patch, as it is essentially a full dump of my `hg diff`. It likely has changes that aren't related to tab support at all. It also contains a lot of debug code that would be removed from the final patch.

However, it does support basic tab functionality (e.g. actually being able to type in the right place after you've inserted a tab), though it does require the 'tabsize' setting to be set to 'tabs'. This will change by the final patch.

This patch currently has some quirks when pasting multiple lines with tabs in it. This is because I haven't yet fleshed out the chunk-handling functions.

Any input is welcome and appreciated.
Gordon, we chatted at length about this in IRC; just leaving this comment for posterity. :-) Essentially, feedback left in chatroom was that:

- this patch pushes tab handling logic back into the model--whereas I just moved to all to a CursorManager abstraction

- I think having the logic encapsulated in the UI layer (CursorManager) makes the most sense in shielding developers from the complexity of tabs

- I'd therefore prefer if you approached this by finishing up the CursorManager migration and getting tab functionality working across actions, etc.

I'll focus on this later this week, but if you beat me to it, huzzah!

Thanks man,

Ben
This doesn't actually work very well with doing anything but adding tabs. The previous patch works much better (i.e. is more feature-complete), but see the discussion on the bespin-core mailing list for more details.
Keywords: student-project
This is essentially just a patch to clean up some code and make things run/look a little smoother. These are things I came across during my attempt to implement tab support and that I fixed along the way. In particular, this patch targets position arrays that are out of order and/or are missing spaces (should be { row, col }). It also fixes the environmental variables that deal with tabs (tabmode and tabsize), and adds some extra, disambiguating parentheses to a complex comparison.
Attachment #368119 - Flags: review?(bgalbraith)
Committed and pushed, but with the two functional changes removed
I think I've finally got it! It's been quite frustrating, but I think this works with everything: manually typing tabs, pasting tabs, pasting chunks with tabs, etc. Undo and redo should also work.

Unless anything comes up that I haven't found, this should be the final patch for tab support, I think
Attachment #367524 - Attachment is obsolete: true
Attachment #367945 - Attachment is obsolete: true
Attachment #368604 - Flags: review?(bgalbraith)
I think I've been successful in establishing a clone/branch/whatever of bespin over on BitBucket, so I've committed my latest code here:
https://bitbucket.org/gphemsley/bespin/changeset/c55548c6940a/

I don't know if that helps at all. It's the same code as attachment 368604 [details] [diff] [review].
Duh. This has been assigned to me for a while.
Status: NEW → ASSIGNED
There are some issues related to selectAll() and deleteKey() in the original changeset.

These problems have been fixed in the following changesets:
https://bitbucket.org/gphemsley/bespin/changeset/7db125f64289/
https://bitbucket.org/gphemsley/bespin/changeset/899116b32dfe/

There are no other changes on this clone, so I believe it can be as easy merging the changes. If not, each changeset has a link to a "raw changeset" which is a patch in diff format.
Because I kinda bungled the merge of the last couple of commits, I've attached the full patch for tabs support, including those aforementioned fixes to selectAll() and deleteKey().
Attachment #368604 - Attachment is obsolete: true
Attachment #369135 - Flags: review?(bgalbraith)
Attachment #368604 - Flags: review?(bgalbraith)
I've applied the patch with some minor fixes to adjust performance.
Closing issue; further bugs with our tab implementation should receive specific bugs
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This is a mass migration from Mozilla Labs :: Bespin to Bespin :: Editor.
Component: Bespin → Editor
Product: Mozilla Labs → Bespin
QA Contact: bespin → editor
Whiteboard: editor
Target Milestone: Future → 0.2.0
Attachment #368119 - Flags: review?(ben)
Attachment #369135 - Flags: review?(ben)
You need to log in before you can comment on or make changes to this bug.