search

postiffm / bibledit-desktop

Desktop version of Bibledit
GNU General Public License v3.0
4 stars 6 forks source link

Use std::map to pair the verse number with the verse class #65

Closed Pickle closed 5 years ago

Pickle commented 5 years ago

This will save some memory in apparatus Allow checking of the source files for repeating verses Allow for proper message if apparatus if the verse does not have one or exceeds the range of verses lised in the apparatus files

Pickle commented 5 years ago

the gui message displayed by the apparatus when a verse is not found is in line 362. Currently it has an out range message, but i think something like "No verse found" could be better.

postiffm commented 5 years ago

Thank you very much for these improvements. I will take a look very soon.

postiffm commented 5 years ago

Good changes, Scott. Thanks. I implemented your suggested change on line 362 and also the conditional assignment to lastVerse in two cases to make sure we avoid segfaults.

Pickle commented 5 years ago

Matt, thanks i meant to getting around and committing that last fix. I also thought of looking into printing an appropriate message if an item does not exist for a verse. If i remember correctly it only shows the textbox as blank.

On Tue, Dec 11, 2018 at 5:48 PM Matt Postiff notifications@github.com wrote:

Good changes, Scott. Thanks. I implemented your suggested change on line 362 and also the conditional assignment to lastVerse in two cases to make sure we avoid segfaults.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/postiffm/bibledit-desktop/pull/65#issuecomment-446392531, or mute the thread https://github.com/notifications/unsubscribe-auth/AAaOZUCQcCGjR3_aHJYafZNZ85o_Lgu2ks5u4DZZgaJpZM4X-R_t .

postiffm commented 5 years ago

Are you thinking about having a specialized message for the various types of content? Like in the base class have a "message_to_print_if_verse_doesnt_exist" and then derived classes can specialize that method to return stuff like "No verse in this translation" or "No apparatus for this verse" or "No cross-reference for this verse" ?

Pickle commented 5 years ago

Yes one thought was to have the specific message for the type of item the class was being used for. I had thought the warning might be in parent class containing the note/apparatus. Your idea of creating extending the base class is probably the proper way. I need to think/review how that would work if there were 2 classes with the generic code.

On Wed, Dec 12, 2018 at 11:05 AM Matt Postiff notifications@github.com wrote:

Are you thinking about having a specialized message for the various types of content? Like in the base class have a "message_to_print_if_verse_doesnt_exist" and then derived classes can specialize that method to return stuff like "No verse in this translation" or "No apparatus for this verse" or "No cross-reference for this verse" ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/postiffm/bibledit-desktop/pull/65#issuecomment-446641932, or mute the thread https://github.com/notifications/unsubscribe-auth/AAaOZetp1sI48AMXSpxt5lj9Y0cb-4xmks5u4SlEgaJpZM4X-R_t .

postiffm commented 5 years ago

As I looked at it, it appears that we don't have much of an issue here. For each tab:

Greek and English: if there is no verse, then the entire line of data disappears and "gets out of the way" of the end user, as when we are looking at an OT book with a version like BYZ that only has NT books.

Apparatus: It says "No verse found." This is fine. We can tweak it later.

Cross References: It says "No cross references" so this message is already "specialized" for the type of data that it is displaying.

Pickle commented 5 years ago

ok im good with leaving as is, thanks for the merge

On Wed, Dec 12, 2018 at 4:54 PM Matt Postiff notifications@github.com wrote:

As I looked at it, it appears that we don't have much of an issue here. For each tab:

Greek and English: if there is no verse, then the entire line of data disappears and "gets out of the way" of the end user, as when we are looking at an OT book with a version like BYZ that only has NT books.

Apparatus: It says "No verse found." This is fine. We can tweak it later.

Cross References: It says "No cross references" so this message is already "specialized" for the type of data that it is displaying.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/postiffm/bibledit-desktop/pull/65#issuecomment-446758483, or mute the thread https://github.com/notifications/unsubscribe-auth/AAaOZWVabpVG3d-1rOJUKAlW_ExqlZrVks5u4Xr9gaJpZM4X-R_t .