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

String editing #95

Open
Kixunil opened this issue Feb 19, 2021 · 5 comments
Open

String editing #95

Kixunil opened this issue Feb 19, 2021 · 5 comments

Comments

@Kixunil
Copy link
Contributor

Kixunil commented Feb 19, 2021

Sooner or later we need to figure out how to do string editing - e.g. "foo" in Json or <foo> in XML.
I only have a rough idea now, but some things that make sense to me:

  • Have a "stringly" node (defined below) that can be edited like a string.
  • Allow languages to define escaping and unescaping functions - then the user can write unescaped text and it will become escaped when leaving string editation mode - this can be incredibly helpful as people often forget the escaping rules of different languages. This feature may be optional but it is probably easier to write it as mandatory for now.
  • I don't believe it's a good idea to represent strings as lists of chars - likely not useful and a waste of memory

The API of stringly node can be described by this trait (which may or may not be an actual trait)

trait StringlyNode {
    // Cow can save some allocations however we should inform the implementors which representation is more efficient.
    // My guess for now is that it'll be more efficient to store escaped version as unescaped one will be only used in string edit mode
    // Or maybe pass fmt::Write?
    fn get_string_unescaped(&self) -> Cow<'_, str>;
    // The user has to escape manually
    fn get_string_escaped(&self) -> Cow<'_, str>;
    // this generic can save some allocations while not caring about `&str` vs `String`
    // returns Err if the string contains banned chars
    // Alternatively we could use Cow to make this object safe
    fn set_string_unescaped<S: Deref<Target=str> + Into<String>>(&mut self, string: S) -> Result<(), SetStringError>;
    fn set_string_escaped<S: Deref<Target=str> + Into<String>>(&mut self, string: S) -> Result<(), SetStringError>;
    // Validates the string, may be called after user typing each char and revert the change if this fn returns false
    fn is_string_valid(&self, string: &str) -> bool;
}

I imagine this flow (pseudo code):

// when string edit mode is entered
// returned value imple StringlyNode
// returns Err if the node is not stringly
let stringly_node = tree.get_stringly_node(cursor)?;
let edited_string = String::from(stringly_node.get_tring_unescaped());
self.mode = Mode::StringEdit { stringly_node, };

// when leaving string edit mode:
match stringly_node.set_string_unescaped() {
    Ok(_) => self.mode = Mode::Normal,
    Err(error) => log_error!(error),
}

OT: so far I didn't have as much time to look at XML as I wished. I'm doing it now and maybe a bit tomorrow but I don't feel that great so I may be unable to finish it.

@Kixunil
Copy link
Contributor Author

Kixunil commented Feb 19, 2021

One more observation: when e.g. replacing XML tag foo with bar the string may be in an intermediate state when it's empty - invalid as far as XML is concerned but it'd be annoying to not allow it. So we shouldn't prevent the user from having invalid string temporarily.

However there's and interesting issue - if string edit mode will work like insert mode in vim then you can't undo until you leave it but you can't leave it in invalid state. Some ways to resolve this:

  • Use Ctrl + something to perform undo
  • String edit mode acts as vim, so effectively there is String edit normal mode and string edit insert mode. This is cool but maybe annoying and complex to implement.
  • When the user leaves string edit mode with invalid string it's in a special state that only allows two operations: undo the edit or re-enter the string edit mode

Also the error returned from validation should contain some kind of span with list of invalid characters.

@Kixunil
Copy link
Contributor Author

Kixunil commented Feb 19, 2021

Extra features stuff: allow all languages to define node tags/discriminants. Then user could s/foo/bar/ all stringly nodes that match some condition (e.g. "is Json key").

@kneasle
Copy link
Owner

kneasle commented Feb 20, 2021

Sooner or later we need to figure out how to do string editing - e.g. "foo" in Json or <foo> in XML.
I only have a rough idea now, but some things that make sense to me:

  • Have a "stringly" node (defined below) that can be edited like a string.

  • Allow languages to define escaping and unescaping functions - then the user can write unescaped text and it will become escaped when leaving string editation mode - this can be incredibly helpful as people often forget the escaping rules of different languages. This feature may be optional but it is probably easier to write it as mandatory for now.

  • I don't believe it's a good idea to represent strings as lists of chars - likely not useful and a waste of memory

Yeah this makes sense - it's pretty much in line of what I was thinking. I'm still not sure of the best way to represent nodes at all, but 'stringly' nodes are so ubiquitous that any decent architecture needs to handle them correctly. My current feeling is that it would be nice to have only one Ast type, which specifies a grammar and rules about which node types are stringly, containers, key-value pairs, etc. But that isn't ideal on several fronts, but it does have nice features like handling whitespace easily, making the rest of Sapling's code simpler, etc.

OT: so far I didn't have as much time to look at XML as I wished. I'm doing it now and maybe a bit tomorrow but I don't feel that great so I may be unable to finish it.

Don't stress about it - there's no pressure. I'm personally quite busy with my degree project right now, and I'm not usually in the mood for making more projects after a day of coding crazy projects in Rust. I really don't want to fully back-burner Sapling, though, so I'll keep streaming and doing stuff albeit slower than usual.

@kneasle
Copy link
Owner

kneasle commented Feb 20, 2021

One more observation: when e.g. replacing XML tag foo with bar the string may be in an intermediate state when it's empty - invalid as far as XML is concerned but it'd be annoying to not allow it. So we shouldn't prevent the user from having invalid string temporarily.

I think this another example of where allowing nodes to be temporarily invalid makes a huge difference to the complexity of the rest of the code (the existing one being how unbelievably janky the existing replace code is).

  • String edit mode acts as vim, so effectively there is String edit normal mode and string edit insert mode. This is cool but maybe annoying and complex to implement.

I suspect this might be the easiest strategy - though I agree it might be hard to understand and implement.

@kneasle
Copy link
Owner

kneasle commented Feb 20, 2021

Extra features stuff: allow all languages to define node tags/discriminants. Then user could s/foo/bar/ all stringly nodes that match some condition (e.g. "is Json key").

I have an rough idea for how to do things like this once multiple cursors are a thing:

  • Select the region of the tree that you want to perform the find/replace in
  • Run a search command to search the subtrees you've selected for the nodes you want to replace. This will create one cursor (which can be thought of as an individual selection) at each node that you are interested in.
  • Switch into string edit mode. This will be editing the strings of all the selected nodes simultaneously
  • Clear all the strings with some key (probably d for delete, since it feels most intuitive for everything to be selected when you go into string mode)
  • Go into string insert mode
  • Type the new name for the nodes
  • Exit string insert mode
  • Exit string mode
  • Perhaps reset back to only one cursor

It seems like a lot of steps, but each step can be reused for a lot of different things (e.g. the first 3 steps can be used to delete all the p tags in a set of subtrees, for example).

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

No branches or pull requests

2 participants