gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.68k stars 690 forks source link

TextView: TextView.LoadFile keeps file open and provides no way of closing it #452

Open tig opened 4 years ago

tig commented 4 years ago

The code in TextModel uses File.OpenRead but there's no code that ever closes it.

As a result, clients can't save the file loaded this way.

tig commented 4 years ago

@BDisp , thanks for your PR #487 where you added a CloseFile method.

I probably should not have accepted the PR because while it meets the spirit of what I noted in this bug, I'm not sure it is actually correct design.

Instead, I think the reading/writing of the file for TextView should be left up to the caller. If it were up to me, I'd remove all file/stream logic from the class. It is trivial for a client to open/read/write the file and just assign it to TextView.Text as I did in the Editor Scenario (take a look).

Client code to Load:

            if (_fileName != null) {
                _textView.Text = System.IO.File.ReadAllText (_fileName);
                Win.Title = _fileName;
                _saved = true;
            }

Client code to Save:

            if (_fileName != null) {
                // BUGBUG: #279 TextView does not know how to deal with \r\n, only \r 
                // As a result files saved on Windows and then read back will show invalid chars.
                System.IO.File.WriteAllText (_fileName, _textView.Text.ToString());
                _saved = true;
            }

Why mess with re-implementing all of what .NET gives us here?

BDisp commented 4 years ago

I only made it because you mentioned somewhere that was missing.

tig commented 4 years ago

I only made it because you mentioned somewhere that was missing.

You are right, I did phrase it in a way that may have sounded like a request for a Close method.

Are you ok with us just closing #487 and then re-visiting how to fix this issue longer term here?

BDisp commented 4 years ago

But you already merged it.

tig commented 4 years ago

Right. Sorry, i have not had enough coffee this AM.

We can just leave it in for now since it does no harm. Ok?