Skip to content

Conversation

axelkar
Copy link

@axelkar axelkar commented Oct 16, 2025

This is useful for collaborative text editing as user intent is very easily lost when just diffing text.

Should #[must_use] be placed on TextModification?

@DJMcNab
Copy link
Member

DJMcNab commented Oct 16, 2025

Hmm. I'm not sure how to reason about whether we want to add this. I do think that in general collaborative editing is probably out of scope for PlainEditor in Parley, at least at the moment. That's not to say that a solution for collaborative editing couldn't use Parley, but that I'd expect a collaborative editor built using Parley to own its own editor component.
This assessment isn't authoritative; there are people involved in Parley's development who likely have a better sense of this than I do.

That being said, I think a very similar approach would be useful for getting basic undo/redo support landed, which I do think would be in scope for PlainEditor (see also linebender/xilem#1417). I can see that in that case you'd also want the modified content (both added and removed, I guess), and to keep track of the cursor position. I think that should be a relatively small addition on top of this code.

If we were to do something like that, I'd like to see basic undo/redo added to the vello_editor example using it. But again, I've not been closely following the work on Parley recently, so I might be misjudging this.

@axelkar
Copy link
Author

axelkar commented Oct 16, 2025

I can see that in that case you'd also want the modified content (both added and removed, I guess), and to keep track of the cursor position.

That wouldn't even need this, or integration in Parley at all. You can just keep a Vec<String> with versions of PlainEditor::text in memory. Just compare and save the text and cursor position after all calls to PlainEditor and PlainEditorDriver. That way the widget library can handle the history retention itself.

As an unrelated question, are custom inline elements supported in PlainEditor's layout? I haven't diven that deep.

@nicoburns
Copy link
Contributor

As an unrelated question, are custom inline elements supported in PlainEditor's layout? I haven't diven that deep.

I believe not. There was some work towards making that possible (mostly around selections and cursors), but it never got as far as actually working.

@DJMcNab
Copy link
Member

DJMcNab commented Oct 20, 2025

We discussed this in office hours, and the consensus position was that this is a reasonable feature to have here.
I'm not sure when we'll get this reviewed, but it is on the radar, and we're broadly in favour. From my quick scan, this PR looks to be a good implementation of the feature, but I don't have the time to do a full review now.

That wouldn't even need this, or integration in Parley at all. You can just keep a Vec<String> with versions of PlainEditor::text in memory. Just compare and save the text and cursor position after all calls to PlainEditor and PlainEditorDriver. That way the widget library can handle the history retention itself.

That feels very memory inefficient to me. Another reason that you might want tighter integration in the editor is so that your history mechanism can e.g. delete characters, then words. But I still agree, that this PR isn't designed for that use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants