gchp / iota

A terminal-based text editor written in Rust
MIT License
1.63k stars 81 forks source link

Remove use of unsafe code #38

Closed gchp closed 9 years ago

gchp commented 9 years ago

Currently there are just two unsafe blocks of code in the project. I'd like to remove them and only use safe code if possible.

I'm not sure if the current design will work without the unsafe code, though. Might be worth evaluating to see if its worth redesigning just to remove it.

Arcterus commented 9 years ago

The problem right now seems to be the reference to a Line stored in Cursor. It's the cause of all the unsafe code at the moment.

gchp commented 9 years ago

Right. So the current cursor design came more or less from the vigo project. It was what I was familiar with as I worked on that project before so was easy enough to implement. I'm sure there are many alternative designs that could be used here instead, just need to find one!

I wonder how it could work if the cursor didn't have a reference to a line. Could we do that and still have the same functionality? On 14 Dec 2014 19:59, "Alex Lyon" notifications@github.com wrote:

The problem right now seems to be the reference to a Line stored in Cursor. It's the cause of all the unsafe code at the moment.

— Reply to this email directly or view it on GitHub https://github.com/gchp/iota/issues/38#issuecomment-66927129.

Arcterus commented 9 years ago

I think one way around the problem would be to transfer most of Cursors current tasks to Buffer. Not sure if we'd want to do that, though.

pythonesque commented 9 years ago

Was https://github.com/gchp/iota/commit/f8d15b675a51a8410242a22f020a93fc53d26f06 the commit that introduced the unsafe? What was wrong with the earlier method?

gchp commented 9 years ago

@pythonesque before that, #32 was happening. I'll be honest, there are times when Rust's lifetimes confuse me, this is one of those times. Can you suggest an alternate fix?

crespyl commented 9 years ago

@Arcterus, that's also kind of what I was thinking.

Hypothetically: Buffer takes responsibility for any alterations to the text, and is passed a line number+offset for each edit. Cursor just needs to know a line number+offset (maybe relative to its parent View?), and View can then figure out the absolute positions to send to the Buffer for editing?

pythonesque commented 9 years ago

I have the fix almost ready.

(I actually had a second solution along those lines, but I opted for a slightly different one to keep the existing code mostly the same).

gchp commented 9 years ago

I've just merged #39 which solved this. I like it because it keeps the editing logic in the cursor module. I think its quite clean this way. The view handles the displaying of the buffer, handling things like scrolling, rendering cursor etc. The buffer then handles all the text, file paths and passes positional information to the cursor which performs the actual editing on the data. The buffer also handles the logic for what editing command should happen. Like which direction to delete, what character to insert etc. The cursor just performs whatever the buffer determines is the correct action.