Possible CharacterMap enhancements (easy)

[humdinger]

This is hrev29833.

A few possible enhancements:

(done) 1. If I scroll down in the character map on the right side, it'd be nice if the current position was reflected on the left "contents" view by updating the selected block "heading".
(done) 2. By moving the font selection into a drop down menu (to the right of the font size?) it would always be visible what font you're examining.
(wontfix) 3. Is the menu item "Show private blocks" really necessary, is there a need to hide those?

4. Blocks that don't contain any characters could be greyed out (in the "contents" list to the left).
   
5. Left-clicking a character could increase its size (x2 ?) for detailed inspection.
   

(done) 6. The selected character could also be shown black on white in front of the "Code:" line at the button. This way you can be sure you didn't accidentally move the mouse to the adjacent character while taking your eyes from it.

7. CharacterMap should node-monitor for updated fonts.

[axeld]

1. Indeed, that would be nice. I'm just too lazy to implement that, though :-)

2. Primarily, CharacterMap is a Unicode character map viewer, not a font inspector, therefore I don't think it's overly important to always see that. Especially once the app_server will support font overlays (ie. that means it will substitute the font to get the character). Although I would have an option to turn off those overlays to really only see the contents of the current font, so maybe it's not that a bad idea...

3. That's a font issue: a font might define private characters, and you are able to see them this way. And yes, it makes sense to hide them by default as this doubles the number of characters, and that makes scrolling harder.

4. This is currently not supported by Haiku (ie. BFont::Blocks() is not yet implemented).

5. Too much effort for little gain, I think.

6. Done.

7. It should do so, yes. Maybe it should just use a lazily built font menu instead.

[humdinger]

Thanks for looking into things.

WRT 2.), me as an end-user, I'd use the app to find nice/weird characters. So seeing at which font I'm looking would be beneficial.
I guess that it's not possible to have the font type also been supplied when drag&dropping a character into an editor? Provided the editor checks for these things.
That's an area that I'd find very interesting: intelligent drag&drop (as flattened BMessage?). Provide all kinds of information, e.g. size, color, type alongside the "normal" data of the character. This of course for all kinds of inter-app data transfer. So you'd have ID3 Tags for mp3s, width/height for images, etc.

[axeld]

You might be right about the font, but well. Maybe someone wants to do that; I don't think it's really needed.

Adding the font information to the text would be possible to do. I guess I could change that when I find the time.

[humdinger]

I have one more thing to add, though I don't know if that's how the app is normally used:

8. A drag&dropped character into CharacterMap could scroll to and highlight the according character in the map. You may see a character and be interested in it's mapping, right?

I dunno, your call. As always. :)

[cyberguijarro]

Hi! this sounds quite boring but actually a nice task to start getting involved in the project.

Let me see what I can do ;)

[cyberguijarro]

Second enhancement (font list as a dropdown menu).

[aldeck]

I tested the patch (even though i had to manually apply the parts that are now using translated strings). I've discussed the resulting change with humdinger and noticed it wasn't too good ui wise. A solution would be to keep the original font menu where it is and simply display the current font as text, replacing the "Font size:" label which doesn't add interesting information.

[dsizzle]

Hi, I'm looking into the items as a first crack at Haiku. It's been a while since I programmed in C++ but I'm looking forward to getting back to it a bit.

While testing I noticed something else to add:

9. The minimum window width should be increased. Currently at minimum width, the left/right side panes jump distractingly as different characters are selected (due to the width of the Code/UTF-8 data)

[dsizzle]

Patch for Enhancement 1

[dsizzle]

seems like Enhancement 8 is already in the code, but I'm not sure how to test it.

[pulkomandy]

Hi,

Patch 1: In the (unlikely) "character not mapped" case, shouldn't SelectblockForCharacter return, rather than break? What happens if you call Select with an invalid block number? If this is done on purpose (didn't check the code, do we have an "unmapped" block at the end of the list?), a comment should be added to remember people reading the code of that. Something like this:

// Character is not mapped, we will scroll to the special "unmapped" block at the end of the list

Patch 2: It looks like the code for handling displayName would be much simpler if you used a BString instead of a char*. I think the " > " is not needed (we use "DejaVu Sans Book" elsewhere, with "DejaVu Sans" the family and "Book" the style).

Thanks for the patches!

[dsizzle]

rewrote C-style string operations for displayName to use BString - apply *after* 3651-Enh2.patch

[dsizzle]

You're welcome for the patches! Thanks for reviewing them pulkomandy.

For Patch 1, the break; actually will exit the for loop, and then fall out of the function SelectBlockForCharacter without calling the select command. It is a stylistic difference, but if prefer a return; then I'm happy to change it.

For Patch 2, I rewrote the C-style string lines to use BString...much much nicer! I don't know how to combine patches so I uploaded a second patch.

I also added a patch for Enhancement 7 - node monitoring font directories to rebuild the font menu on the fly. It monitors B_SYSTEM_FONTS_DIRECTORY and B_USER_FONTS_DIRECTORY, which seemed correct but the system font dir is read-only, and the user font dir has a slightly different path (that doesn't exist in my installation). Are those appropriate directories to monitor?

[dsizzle]

dang, I accidentally left a debug printf in line 330 of 3651-Enh7.patch ...should I make a patch for that too?

[dsizzle]

I added a patch for Enhancement 9, setting minimum width of character view pane so it doesn't jump when the code line gets too long. This problem can be seen if you shrink the window horizontally, and then navigate to higher code blocks like Domino tiles. In this patch I included the removal of the debug print accidentally left in patch 3651-Enh7; I hope that's ok.

[pulkomandy]

Hi, You can use git rebase -i to edit your git history and merge or split patches. This would be cleaner and would make them easier to review.

Patch 1 applied in hrev48371.

I took another look at patch 2 and I see another strange thing : you have added a call to fFontSizeSlider->SetLabel(displayName); right after creating the slider with that name already, why is that needed?

On patch 7: unfortunately the list of directories to monitor is a bit bigger. You should use the newly introduced BPathFinder class to get the directories (headers/os/storage/PathFinder.h).

Patch 9: this assumes "w" is the largest character, which may be true only for latin fonts. I'm not sure there is a better solution, however. Also the patch now depends on patch 7 so I can't apply it.

[dsizzle]

Thanks, I'll look into git rebase.

patch 2: You're right, that fFontSizeSlider->SetLabel(displayName); is not needed. I think it's a copy/paste error.

patch 7: Thanks, I will have a look at BPathFinder!

If i can figure out the rebasing I will redo patch 2 to remove the extra line and merge the 2 patches, as well as fixing patch 9 to not depend on patch 7.

Thanks for your patience; I'm not used to making patches. I need to be more careful with these!

[dsizzle]

Consolidated patch for enhancement 2

[dsizzle]

patch for enhancement 9 (character view pane size changing rapidly)

[dsizzle]

I looked into the node monitoring some more. It appears that FontManager is the key, as it is already monitoring for fonts in directories and handling the addition and deletion of them. I don't think it's worth it to duplicate the functionality of FontManager. I could add an accessor function to return FontManager's list of font directories and then monitor those but again I'm not sure if that falls into the scope of this issue. Unless I am missing something, I don't see how BPathFinder would find the font dirs. Thoughts? Alternatively I could add a couple more system font dirs but it wouldn't be as robust as FontManager.

[pulkomandy]

One of the directory constants for BPathFinder is B_FIND_PATH_FONTS_DIRECTORY (defined in FindDirectory.h). The BPathFinder APIs will return a complete list of all font directories in the system, and if we add more, it will return those as well.

For enumerating fonts, the existing APIs can be used (see for example src/preferences/appearance/FontMenu.cpp). As Axel suggested, maybe a lazily built menu would be the simplest solution. To implement this, you would use the "MenusBeginning" hook function (in BWindow) to recreate the whole font menu. This way, there is no need to node monitor anything and you can just get the current list of fonts from the FontManager, like the Appearance preferences do.