search

adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.24k stars 7.63k forks source link

Redo on "Beautify" extension command shows a blank editor until you scroll the editor window. #9431

Open RaymondLim opened 10 years ago

RaymondLim commented 10 years ago
  1. Install Beautify extension.
  2. Open index.html from the default Getting Started project.
  3. Select Beautify from Edit menu (or editor context menu).
  4. Ctrl/Cmd+Z to undo.
  5. Ctrl/Cmd+Y to redo.

Result: The editor window becomes blank. I can reproduce it in release 43, but not in release 42. So I don't think this is the extension issue.

RaymondLim commented 10 years ago

git bisect points to this commit 8e8d1b7e8f79530d1.

RaymondLim commented 10 years ago

@redmunds Looking at your changes in the commit, it doesn't seem like this is the real culprit. See you can locate the real one.

redmunds commented 10 years ago

@RaymondLim The result in release 42 is a little different, but it doesn't really work either. Redo in 42 fails (does nothing), so it's hard to tell if the other problem is there or not. I'm seeing this exception in the console:

Uncaught Error: There is no line 203 in the document. codemirror.js:6537

The problem seems to be a painting error. If you click in editor or hit any key, then it paints. Sometimes in 42, the editor doesn't paint after undo.

redmunds commented 10 years ago

The uncaught error mentioned above still happens in Sprint 40. This bug does not happen in Sprint 38. Using this scenario, undo doesn't work in Sprint 39, so I'm not sure how to narrow it down after that.

redmunds commented 10 years ago

@RaymondLim According to git bisect, the uncaught error during redo was introduced in commit https://github.com/adobe/brackets/commit/89065448414a4967c3c7c61c73433fe837b2e6b7. Hopefully fixing that also fixes original bug.

RaymondLim commented 10 years ago

@redmunds Thanks for looking into this. You're right that this only works correctly in sprint 38, but not in any sprint or release after that. I looked into Beautify code and it is calling doc.batchOperation() with that put doc.setText(formattedText), editor.setCursorPos(cursor) and editor.setScrollPos(scroll.x, scroll.y). And I can see that cursor position and scroll position may not be exactly the same in the formatted text. So I'm not sure this is the extension issue or the underlying CM issue.

redmunds commented 10 years ago

I'm not sure how much time we should put into this for Brackets 1.0, so adding Needs Review.

dangoor commented 10 years ago

Reviewed low priority for the Editor feature area.