HaikuArchives / TakeNotes

Complex note-taking application
12 stars 15 forks source link

Create a single open file panel and make sure there's only one window per file #38

Closed fantoro closed 4 years ago

fantoro commented 4 years ago

I moved the open file panel to NoteApplication and checked if the file pointed by the entry_ref is already opened in one of the windows and in case it is I made it open that specific window in NoteApplication::OpenNote(entry_ref *ref). I also replaced the variables holding the count of all windows with a BObjectList that holds the windows to iterate through them in NoteApplication::OpenNote(entry_ref *ref).

Fixes #37

owenca commented 4 years ago

I think it'd be better to use BApplication::WindowAt with a new field entry_ref* fRef for the NoteWindow class. It's a good time to get rid of the scheme of "Untitled Note 0", "Untitled Note 1", etc. and just use (and reuse) "Untitled" when opening an empty NoteWindow, with its fRef set to NULL. (You can have multiple "Untitled" NoteWindows.)

fantoro commented 4 years ago

I think it'd be better to use BApplication::WindowAt with a new field entry_ref* fRef for the NoteWindow class. It's a good time to get rid of the scheme of "Untitled Note 0", "Untitled Note 1", etc. and just use ~(and reuse)~ "Untitled" when opening an empty NoteWindow, with its fRef set to NULL. (You can have multiple "Untitled" NoteWindows.)

Oh, I didn't notice BApplication::WindowAt when reading the docs at first, I'll change the code to use it then. Also I'm not sure if there's actually a point in having that entry_ref* fRef since fSaveMessage already holds the info about the current file.

Just noticed that BApplication::CountWindows() counts other windows as well, should I loop through all of them and count the amount of NoteWindows in NoteApplication::CloseNote? Ok just thought to try to do that but I couldn't think of a good way to actually check if a BWindow is a NoteWindow and diving the number of windows by the number of windows per one NoteWindow doesn't seem like a good idea since the number of windows per one NoteWindow doesn't appear to be that stable.

owenca commented 4 years ago

Ok just thought to try to do that but I couldn't think of a good way to actually check if a BWindow is a NoteWindow and diving the number of windows by the number of windows per one NoteWindow doesn't seem like a good idea since the number of windows per one NoteWindow doesn't appear to be that stable.

You can use dynamic_cast:

NoteWindow* noteWindow = dynamic_cast<NoteWindow*>(WindowAt(index));
if (noteWindow != NULL) {
    // it's a NoteWindow
}
owenca commented 4 years ago

Oh, I didn't notice BApplication::WindowAt when reading the docs at first, I'll change the code to use it then. Also I'm not sure if there's actually a point in having that entry_ref* fRef since fSaveMessage already holds the info about the current file.

It's more efficient and reliable to add a new entry_ref* fRef field to NoteWindow and initialize it to the argument of the constructor (or NULL for the default constructor).

owenca commented 4 years ago

Just noticed that BApplication::CountWindows() counts other windows as well, should I loop through all of them and count the amount of NoteWindows in NoteApplication::CloseNote?

The current way of keeping a count of open NoteWindows is more efficient although the count should be changed to a static (class-wide) field.

owenca commented 4 years ago

You can use Koder (and turn on whitespace viewing) to spot whitespace violations to Haiku Coding Guidelines. Please squash your commits.

fantoro commented 4 years ago

Also, before setting the title in NoteWindow::Save(), update fRef to that of directory/name.

Alright done.

fantoro commented 4 years ago

Really nice barring some crashes which I am almost certain are caused by the scroll view.

By the way, your repo is 1 commit behind now.

Alright, I rebased the repo.

owenca commented 4 years ago

Please remove fullRef.