{"action":"created","issue":{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/367","repository_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine","labels_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/367/labels{/name}","comments_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/367/comments","events_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/367/events","html_url":"https://github.com/ckeditor/ckeditor5-engine/issues/367","id":149431050,"number":367,"title":"Implement rename operation","user":{"login":"Reinmar","id":156149,"avatar_url":"https://avatars.githubusercontent.com/u/156149?v=3","gravatar_id":"","url":"https://api.github.com/users/Reinmar","html_url":"https://github.com/Reinmar","followers_url":"https://api.github.com/users/Reinmar/followers","following_url":"https://api.github.com/users/Reinmar/following{/other_user}","gists_url":"https://api.github.com/users/Reinmar/gists{/gist_id}","starred_url":"https://api.github.com/users/Reinmar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Reinmar/subscriptions","organizations_url":"https://api.github.com/users/Reinmar/orgs","repos_url":"https://api.github.com/users/Reinmar/repos","events_url":"https://api.github.com/users/Reinmar/events{/privacy}","received_events_url":"https://api.github.com/users/Reinmar/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/labels/module:model","name":"module:model","color":"fef2c0"},{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/labels/status:confirmed","name":"status:confirmed","color":"e6e6e6"},{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/labels/type:feature","name":"type:feature","color":"84b6eb"},{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/labels/%E2%98%85%E2%98%85%E2%98%85","name":"★★★","color":"ffdd33"}],"state":"open","locked":false,"assignee":{"login":"scofalik","id":1502228,"avatar_url":"https://avatars.githubusercontent.com/u/1502228?v=3","gravatar_id":"","url":"https://api.github.com/users/scofalik","html_url":"https://github.com/scofalik","followers_url":"https://api.github.com/users/scofalik/followers","following_url":"https://api.github.com/users/scofalik/following{/other_user}","gists_url":"https://api.github.com/users/scofalik/gists{/gist_id}","starred_url":"https://api.github.com/users/scofalik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scofalik/subscriptions","organizations_url":"https://api.github.com/users/scofalik/orgs","repos_url":"https://api.github.com/users/scofalik/repos","events_url":"https://api.github.com/users/scofalik/events{/privacy}","received_events_url":"https://api.github.com/users/scofalik/received_events","type":"User","site_admin":false},"assignees":[{"login":"scofalik","id":1502228,"avatar_url":"https://avatars.githubusercontent.com/u/1502228?v=3","gravatar_id":"","url":"https://api.github.com/users/scofalik","html_url":"https://github.com/scofalik","followers_url":"https://api.github.com/users/scofalik/followers","following_url":"https://api.github.com/users/scofalik/following{/other_user}","gists_url":"https://api.github.com/users/scofalik/gists{/gist_id}","starred_url":"https://api.github.com/users/scofalik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scofalik/subscriptions","organizations_url":"https://api.github.com/users/scofalik/orgs","repos_url":"https://api.github.com/users/scofalik/repos","events_url":"https://api.github.com/users/scofalik/events{/privacy}","received_events_url":"https://api.github.com/users/scofalik/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/milestones/3","html_url":"https://github.com/ckeditor/ckeditor5-engine/milestones/iteration%203","labels_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/milestones/3/labels","id":1834350,"number":3,"title":"iteration 3","description":"Editor features stabilization.\n\nMain goals:\n\n* Fixing most important issues in existing features.\n* Compatibility with other browsers (so far CKEditor 5 was tested only in Chrome).","creator":{"login":"pjasiun","id":1166967,"avatar_url":"https://avatars.githubusercontent.com/u/1166967?v=3","gravatar_id":"","url":"https://api.github.com/users/pjasiun","html_url":"https://github.com/pjasiun","followers_url":"https://api.github.com/users/pjasiun/followers","following_url":"https://api.github.com/users/pjasiun/following{/other_user}","gists_url":"https://api.github.com/users/pjasiun/gists{/gist_id}","starred_url":"https://api.github.com/users/pjasiun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pjasiun/subscriptions","organizations_url":"https://api.github.com/users/pjasiun/orgs","repos_url":"https://api.github.com/users/pjasiun/repos","events_url":"https://api.github.com/users/pjasiun/events{/privacy}","received_events_url":"https://api.github.com/users/pjasiun/received_events","type":"User","site_admin":false},"open_issues":16,"closed_issues":1,"state":"open","created_at":"2016-06-17T11:17:04Z","updated_at":"2016-08-11T09:34:48Z","due_on":null,"closed_at":null},"comments":6,"created_at":"2016-04-19T11:30:45Z","updated_at":"2016-08-11T13:28:43Z","closed_at":null,"body":"Needed for cases like:\r\n\r\n* enter inside a fully selected non-paragraph block (should result in empty paragraph),\r\n* switching between formats,\r\n* converting paragraphs to list items.\r\n\r\nIt could be done in two steps:\r\n\r\n* [x] Implement delta application logic.\r\n* [ ] Add OT support for that delta. Eventually add also the rename operation which will solve issues like:\r\n  * you lose reference to the element being renamed when renaming it (if it's implemented as a delta - insert+move+remove),\r\n  * OT conflicts. \r\n\r\nCons:\r\n\r\n* We're unsure whether this delta makes semantic sense. When changing formats there's a question what to do with attributes. The attributes of the previous element may not make sense on the new element. So is it really semantical renaming if issues like this may appear?\r\n\r\nPros:\r\n\r\n* Such delta will resolve some cases in OT. Especially in the p->li case where many elements need to be renamed at once, doing this with insert+move+remove could create tricky situations."},"comment":{"url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/comments/239160535","html_url":"https://github.com/ckeditor/ckeditor5-engine/issues/367#issuecomment-239160535","issue_url":"https://api.github.com/repos/ckeditor/ckeditor5-engine/issues/367","id":239160535,"user":{"login":"scofalik","id":1502228,"avatar_url":"https://avatars.githubusercontent.com/u/1502228?v=3","gravatar_id":"","url":"https://api.github.com/users/scofalik","html_url":"https://github.com/scofalik","followers_url":"https://api.github.com/users/scofalik/followers","following_url":"https://api.github.com/users/scofalik/following{/other_user}","gists_url":"https://api.github.com/users/scofalik/gists{/gist_id}","starred_url":"https://api.github.com/users/scofalik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scofalik/subscriptions","organizations_url":"https://api.github.com/users/scofalik/orgs","repos_url":"https://api.github.com/users/scofalik/repos","events_url":"https://api.github.com/users/scofalik/events{/privacy}","received_events_url":"https://api.github.com/users/scofalik/received_events","type":"User","site_admin":false},"created_at":"2016-08-11T13:28:43Z","updated_at":"2016-08-11T13:28:43Z","body":"Renaming appears to be more tricky than I thought. We have to consider few things if go with `RenameOperation`. Changes in model are easy. It's just `element.name = newName;`, done. Change has `rename` type. Problems start during conversion.\r\n\r\nWe could \"close\" the case right in `ModelConversionDispatcher`. For `rename` change, we could fire `convertRemove()` and `convertInsertion()`. This solution is very clean. First of all, we do not introduce any new events for conversion. Second of all, we use already defined callbacks (keep in mind that some of them might be custom so it's good that they got reused and don't have to be tweaked for different kind of event). Third, changing element name may, theoretically, have an impact on how that element is rendered or how attributes for that element work or how it's children are rendered. In this approach, we re-render all the children so it's clean.\r\n\r\nThe drawback is that if big element gets changed or many element get changed, we re-render a lot of stuff. There is also a problem with any embeded content, like MathJax or anything we use iframes for, because they will be reloaded.\r\n\r\nSo next idea was to do `convertInsertion()`, `convertMove()` and `convertRemove()` for `rename` change type. Insertion convertion would be only on the renamed element (not it's children). Then children are moved from \"old\" element to \"new\", and then \"old\" element is removed. However I fear that there might be some problems with model - view mappings, because `convertInsertion()` will make some mappings, so same instance of model will be mapped to two elements in view.\r\n\r\nAnother idea was to implement `viewWriter.rename()` that would take responsibility of renaming given element. Then model-to-view converter for `rename` change type would be just firing that writer method. However, that method cannot just change element name because that might result in incorrect conversion. We would have to insert new view element and remove old. But then we don't know in the writer what kind of element we should create and how. So in fact, we still need `convertInsertion()` or `rename` converter would have to be more complicated.\r\n\r\nAnother problem is schema corectness. Model has to be always correct according to schema. So I propose that `RenameOperation` will:\r\n1. remove all attributes from the renamed element that are not correct with schema,\r\n2. remove all children from the renamed element that are not correct with schema.\r\n\r\nSo that would sum up some problems with rename operation. I think that I will try to come up with an approach that does not re-render children of renamed element. It will be probably some variation of solution with `convertInsertion()`, `convertMove()`, `convertRemove()` approach. I don't know whether I will do these operations in `ModelConversionDispatcher` or whether I will introduce `rename` event."}}
{"actor":{"display_login":"scofalik"}}