Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set value issue #590

Merged
merged 3 commits into from Jul 20, 2020
Merged

Set value issue #590

merged 3 commits into from Jul 20, 2020

Conversation

zTree
Copy link
Contributor

@zTree zTree commented Jul 17, 2020

之前我改造的 setValue 方法,增加 clearStack 参数时,对 undo 了解不够,enableAddUndoStack 设置为 !clearStack;
这样会导致 clearStack 设置为 true 时, undo 异常,所以需要始终设置为 true。

@Vanessa219
Copy link
Owner

Vanessa219 commented Jul 18, 2020

既然清空了,为什么还要入栈呢?

是在什么场景下会有问题呢?我试了下,没试出来~~~

@zTree
Copy link
Contributor Author

zTree commented Jul 20, 2020

按照当前逻辑,bug 的现象如下:

  1. 切换笔记时,设置新笔记的内容:setValue('abc', true);
  2. 按照当前逻辑, setValue 后不会执行 addToUndoStack,且 栈 的长度为 0
  3. 开始编辑笔记,例如 在 'abc' 后面输入 'de';等一会儿,保证 addToUndoStack 执行,然后执行 undo,会发现无效,无法让笔记执行 undo 操作回到 原始的 'abc' 状态

分析产生原因:

  1. 初始化 Vditor 后,会执行一次 addToUndoStack,把笔记初始内容在 undo 中入栈
  2. 执行 setValue('abc', true) 时,对于 undo 来说仅仅清空了栈,这时候,并没有执行 addToUndoStack,就相当于 undo 的栈内并未保留笔记原始状态。
  3. 还有一个非常关键的 recordFirstPosition ,我个人认为其实真正的罪魁祸首在这里,但是这个在你的代码中的作用应该比较重要,我目前不敢擅自改动。这里应该是为了记录第一次修改前的光标位置;但正是这里判断每次 栈的长度为 1 时,就会直接把当前内容去替换 this[vditor.currentMode].undoStack[0][0].diffs[0][1] 的内容(正是因为这个逻辑,导致的 undo 无效)

如果从 setValue 执行后,每次编辑后都执行一次 Ctrl+Z,你会发现 undo 就废了。

所以,在不修改你整体 undo 栈操作逻辑的情况下,直接在 setValue 内设置 enableAddUndoStack 为 true,就相当于每次初始化后的第一次 addToUndoStack 了,整体操作就不会受影响了

@Vanessa219

@Vanessa219
Copy link
Owner

非常感谢你的详细说明。

现在还有个问题,clearStack 是做为一个公开的的法,如果直接调用的话,可能也会出现以上你所说的问题。

是否在 clearStack 的具体实现中不应该清空所有的栈,永久保留第一个是否会好一点?

@zTree
Copy link
Contributor Author

zTree commented Jul 20, 2020

感觉你可能没有理解我的应用场景:

  1. 从笔记列表选择 ”笔记A“,然后执行 setValue('abc', true);
  2. 从笔记列表选择 ”笔记B“,然后执行 setValue('xyz', true);
    ......

所以我的场景下,每次更换笔记,都要重新设置内容,并且保证初始的 undo 为空,不能说 打开 笔记B 的时候,undo 操作后,居然变成了 笔记A 的内容。 这是坚决不允许的,所以每次 setValue 后,都必须要清空 undo 的。

我想了解一下,你那边的想法,对于 clearStack 为 true 时,不清空 所有栈 还有其他什么顾虑么?

@Vanessa219
Copy link
Owner

哦,我明白了。我没考虑到你说的应用场景。想到的是另外一个相关的 API。没有问题。

@Vanessa219 Vanessa219 merged commit 3f592e0 into Vanessa219:dev Jul 20, 2020
@Vanessa219 Vanessa219 self-requested a review July 20, 2020 03:25
@Vanessa219 Vanessa219 added this to the 3.3 milestone Jul 20, 2020
Vanessa219 added a commit that referenced this pull request Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants