Scrolling text in Input widget

[prl]

This alpha allows the Input widget (used for the text entry display in VirtualKeyBoard and InputBox) to scroll the text if it is too wide to fit in the space available, rather than having the cursor disappear out of the box when moved too far along the text string.

The Input widget can already scroll, by setting the parameter visible_width, but that sets the number of characters displayed and doesn't work well with variable-width fonts where an "i" occupies much less width than a "W".

It fixes Bug #317: Unable to edit long file names, but will also fix some other similar problems in VirtualKeyBoard uses, and has the potential for wider application in some text input popups that currently use visible_width.

The new functionality can be tested in the VirtualKeyBoard popup for the rename function in the media selection screen (where the bug was first noticed) - from live TV MEDIA, navigate to recording/file, long-RED. It can also be tested in the timer edit screen. From live TV TIMER, navigate to timer, OK, then press TEXT when either Name or Description is in focus.

It is also useful to test the old functionality, e.g. for the parental control PIN in MENU>Setup>TV>Parental control, and for scrolling with a fixed character width in File Commander. From live TV. MENU>Sources / Files, navigate to file, BLUE Rename.

The changes should work for all non-wrapping alignments, right, left or center. It can be tested with right alignment in the builtin skins, and with left alignment in OverlayHD.

Uninstall this patch before doing any online firmware update! A firmware update from USB will safely remove this patch, and recover from any errors you might make in doing the install or uninstall.

This patch was tested on firmware 20180828beta, but it will probably work on even quite old firmware.

It is compatible with OverlayHD and it should also co-exist with Patches By Adoxa.

If you're uncomfortable with doing any part of the instructions, don't do any of them!

To install:

To apply the patches, download the linked .ZIP file, and extract it. It will create a new directory/folder called input-scroll-installer, which contains two files, installer.sh and uninstaller.sh.

Copy the two files somewhere convenient on a T/U series box (like /home/root), then log into the box using telnet or ssh, change directory to the place you put the installer.sh/uninstaller.sh files. If you put the files in /home/root you'll be in the right place as soon as you log in.

In recent betas, you can also install and uninstall by running the scripts from inside File Commander.

To install the patches run:

sh installer.sh

and restart the GUI (or reboot).

To uninstall the patches, log in and go to the directory as you did to install, and run

sh uninstaller.sh

And restart the GUI.

Make sure you uninstall before doing an upgrade. Don't run the installer if you've already installed & don't run the uninstaller if you've already uninstalled.

You can check whether the patches are installed by logging in and running this on the box:

find /usr/bin /usr/share/enigma2 /usr/lib/enigma2 -name \*.bak

It should print nothing if the patch isn't installed, and it should print

Code: Select all

/usr/lib/enigma2/python/Components/Input.pyo.bak

Comments welcome!

[IanSav]

Hi Prl,

How long will your testing window be available? I have commitments to the other images I need to complete but this change is of great interest for me.

Regards,
Ian.

[prl]

The patch only changes Components.Input, and I don't anticipate any conflicting changes in that any time soon. I'd rather wait for your input than push for an early deadline.

[IanSav]

Hi Prl,

I will push through the tests I promised for OpenViX today and then jump back to test you new code. I think the OpenPLi people will be okay waiting for the fixes their code needs.

Thanks.

Regards,
Ian.

[adoxa]

Code: Select all

			# Only recaclulate the character picel positions

Two typos in one line.

Moving to the right the cursor still disappears occasionally (Ian's Virtual Keyboard, FMW, timer description of Pointless).

getPadding is a relative constant, so could probably be done once in applySkin.

I'll stick with my own solution, after all.

[prl]

Code: Select all

			# Only recaclulate the character picel positions

Two typos in one line.

Thanks.

Moving to the right the cursor still disappears occasionally (Ian's Virtual Keyboard, FMW, timer description of Pointless).

FTA or IceTV description text?

getPadding is a relative constant, so could probably be done once in applySkin.

No, getPadding()'s padding pixel width can change if the font changes (size or face). What's really missing is a force argument to update().

[adoxa]

Moving to the right the cursor still disappears occasionally (Ian's Virtual Keyboard, FMW, timer description of Pointless).

FTA or IceTV description text?

FTA.

getPadding is a relative constant, so could probably be done once in applySkin.

No, getPadding()'s padding pixel width can change if the font changes (size or face). [...]

But if the font could change that would mean getTextPixelPos should always be recalculated, too.

[prl]

getPadding is a relative constant, so could probably be done once in applySkin.

No, getPadding()'s padding pixel width can change if the font changes (size or face). [...]

But if the font could change that would mean getTextPixelPos should always be recalculated, too.

Hence the part of my post that's left out of the quote: "What's really missing is a force argument to update()." The intention would be to force recalculation of both the padding and the pixel position array (which would mean a force argument to getTextPixelPos(), too.

[prl]

Moving to the right the cursor still disappears occasionally (Ian's Virtual Keyboard, FMW, timer description of Pointless).

I can reproduce the problem on the FTA "Pointless" timer description in the distributed Virtual Keyboard and the easy-skin-aus-hd skin, too.

[prl]

Moving to the right the cursor still disappears occasionally (Ian's Virtual Keyboard, FMW, timer description of Pointless).

I can reproduce the problem on the FTA "Pointless" timer description in the distributed Virtual Keyboard and the easy-skin-aus-hd skin, too.

I think I've found a fix for this, but it would be great for testing if anyone's found other strings that have similar problems.

[prl]

IanSav, when I was testing the Input changes on OverlayHD on editing a timer description, I noticed that the header widget skin for VirtualKeyBoard has a height that is not quite a multiple of the text height. I think it should probably be 46px high rather than 50px.

[IanSav]

Hi Prl,

IanSav, when I was testing the Input changes on OverlayHD on editing a timer description, I noticed that the header widget skin for VirtualKeyBoard has a height that is not quite a multiple of the text height. I think it should probably be 46px high rather than 50px.

Thanks for the report. The new VirtualKeyBoard skin has a "prompt" height of 50 pixels that are two lines of font height 20 pixels.

Now that OpenPLi and OpenViX have moved / are moving to my new code I don't know what to do about updating OverlayHD for Beyonwiz. All my code was updated in expectation that the code would not be rejected. As frustrating this is for users it is significantly more frustrating for me.

Regards,
Ian.

[prl]

Hi Prl,

IanSav, when I was testing the Input changes on OverlayHD on editing a timer description, I noticed that the header widget skin for VirtualKeyBoard has a height that is not quite a multiple of the text height. I think it should probably be 46px high rather than 50px.

Thanks for the report. The new VirtualKeyBoard skin has a "prompt" height of 50 pixels that are two lines of font height 20 pixels.

Yes, I know. But two lines text with font height 20 occupies 46 pixels, not 50, and the extra 4 pixels height shows the tops of the risers in the third line if there is one:

[prl]

I've replaced the patch ZIP in the original post with input-scroll-installer1.zip, which should fix the "randomly hiding cursor" problem that adoxa found.

You need to uninstall the old patch (if installed) before installing the new one.

[adoxa]

Did you forget to remove getPadding from update?

[IanSav]

Hi Prl,

Yes, I know. But two lines text with font height 20 occupies 46 pixels, not 50, and the extra 4 pixels height shows the tops of the risers in the third line if there is one:
Screen Shot 2018-09-01 at 13.01.34.png

Believe it or not, I actually do this deliberately with the thought that it is handy/interesting to note when there may be text that you can't see in some places. This usually prompts me to adjust the text or sizing of the control to reduce or eliminate the issue. Without the glimpse these issues usually go unnoticed.

Is this a bad habit that you would suggest I ditch?

Regards,
Ian.

[prl]

Hi Prl,

Yes, I know. But two lines text with font height 20 occupies 46 pixels, not 50, and the extra 4 pixels height shows the tops of the risers in the third line if there is one:
Screen Shot 2018-09-01 at 13.01.34.png

Believe it or not, I actually do this deliberately with the thought that it is handy/interesting to note when there may be text that you can't see in some places. This usually prompts me to adjust the text or sizing of the control to reduce or eliminate the issue. Without the glimpse these issues usually go unnoticed.

Is this a bad habit that you would suggest I ditch?

It's not what I'd do. I think it looks untidy, but I can see your point.

[prl]

Did you forget to remove getPadding from update?

Indeed I did. Fixed now, thanks. Have you noticed any issues with the display?

[adoxa]

With right alignment moving to the left may sometimes add characters that would have fitted (presumably the same would happen with left alignment, moving to the right). Moving to the right sometimes the cursor skips - I don't think it's necessary to always have the NBSP, it's only needed at the very end. I won't provide specific details, as I'll see about modifying eLabel itself, to make the marked position visible (my way, keeping it in the centre when possible).

[prl]

With right alignment moving to the left may sometimes add characters that would have fitted (presumably the same would happen with left alignment, moving to the right). Moving to the right sometimes the cursor skips - I don't think it's necessary to always have the NBSP, it's only needed at the very end. I won't provide specific details, as I'll see about modifying eLabel itself, to make the marked position visible (my way, keeping it in the centre when possible).

When the cursor moves left, the code simply makes the offset equal to the cursor if the offset is greater than the cursor (when moving using CH+/-, that will always just be a step of one for both variables). I'm not sure exactly what you're describing there.

The problem with my approach is that when the cursor moves right, it calculates back from the cursor position to find the first character that can be fitted in the window and still display the cursor, it then calculates forward to find how many characters can fit from the chosen offset. That may be past the cursor position in some cases. It's difficult in general to avoid that problem while keeping the property that the implementation has of not moving the offset or the last character displayed if the cursor moves within the already-displayed characters.

The Input code has no knowledge of the text alignment.

The cursor does not skip, and update() does not modify self.currPos (which would definitely be wrong).

[prl]

I think I may have a fix for the "hanging last character on cursor move right" problem.

[adoxa]

I've modified eTextPara::realign to keep the marked position in the centre. It worked well with FMW (right alignment), but not with OverlayHD (left alignment). If anyone would like to test it once I sort that out, let me know which box(es) you want.

[prl]

I've modified eTextPara::realign to keep the marked position in the centre. It worked well with FMW (right alignment), but not with OverlayHD (left alignment). If anyone would like to test it once I sort that out, let me know which box(es) you want.

I don't think I like that approach much (though without testing i it's hard to be sure).

[prl]

There's another bug in the last released alpha of Input. If you move to somewhere in the middle of the rightmost window into the input text and use "delete right" (NEXT in the distributed VirtualKeyBoard code, and PLAYPAUSE in IanSav's new version of VirtualKeyBoard), then the offset for the displayed text isn't updated correctly and the displayed text shrinks from the left inwards.

I discovered this while I was testing the "hanging last character on cursor move right" problem.

[adoxa]

I've modified eTextPara::realign to keep the marked position in the centre.

I don't think I like that approach much (though without testing i it's hard to be sure).

Which, doing it in realign or keeping the mark centred? I think both are great. Doing it in realign means Input just calls setMarkedPos and that's it (i.e. no change, realign takes care of the position and clipping, with much simpler code). Keeping the mark centred means you get some context.

[prl]

I've modified eTextPara::realign to keep the marked position in the centre.

I don't think I like that approach much (though without testing i it's hard to be sure).

Which, doing it in realign or keeping the mark centred? I think both are great. Doing it in realign means Input just calls setMarkedPos and that's it (i.e. no change, realign takes care of the position and clipping, with much simpler code). Keeping the mark centred means you get some context.

Keeping the mark centred. I prefer that moving the cursor within the visible area doesn't scroll.

[prl]

I've modified eTextPara::realign to keep the marked position in the centre. It worked well with FMW (right alignment), but not with OverlayHD (left alignment). If anyone would like to test it once I sort that out, let me know which box(es) you want.

Yes, please (U4 preferred), or I can possibly download the changed source and build my own.

[adoxa]

Now that I've pushed it I'd prefer you build your own, but if other U4 owners want to test, speak up and I'll build it tomorrow.

[prl]

I've built a U4 enigma2 with adoxa's changes to eLabel & eTextPara. I'll give it a whirl tomorrow.

One side-effect of those changes is that I think it will also fix the problem that IanSav describes in the comments part of Bug #317: Unable to edit long file names.

[prl]

I've built a U4 enigma2 with adoxa's changes to eLabel & eTextPara. I'll give it a whirl tomorrow.

One side-effect of those changes is that I think it will also fix the problem that IanSav describes in the comments part of Bug #317: Unable to edit long file names.

Adoxa's changes result in a fix for Bug #317 in screens that use the Input widget (e.g. VirtualKeyBoard and InputBox). Disappointingly, they don't fix similar problems in ConfigText widgets, because their representation is via eListboxPythonConfigContent, a subclass of eListboxPythonStringContent, rather than eLabel. I'd forgotten that

They do fix the issue in the SMS-input information panel (NumericalTextInputHelpDialog) that IanSav raised in the comments in the issue tracker entry for Bug #317.

As I expected, I don't particularly like the scrolling UI.

I've also found a puzzling problem in 20180901 with the TimerEntry screen (from live TV, TIMER, navigate to timer, OK). If I navigate to either Name or Description, LEFT and RIGHT open the VirtualKeyBoard screen instead of moving the ConfigText's cursor right and left. I can't (for the moment) work out what's causing it.

[adoxa]

Disappointingly, they don't fix similar problems in ConfigText widgets, because their representation is via eListboxPythonConfigContent, a subclass of eListboxPythonStringContent, rather than eLabel. I'd forgotten that

I think this might be better fixed by always using (a possibly calculated) visible_width (I tested with the EPG cache name, and that displayed right over top of the description). Is there another setting I should test?

They do fix the issue in the SMS-input information panel (NumericalTextInputHelpDialog) that IanSav raised in the comments in the issue tracker entry for Bug #317.

Ah, now I see what that is. I only anticipated single lines, I'll see if I can make it work with multiple.

As I expected, I don't particularly like the scrolling UI.

Moving the cursor would mean having to keep track of where the text is. Obviously I'm happy as it is, so I'll leave that for you (I'll make it an option if you don't).

I've also found a puzzling problem in 20180901 with the TimerEntry screen (from live TV, TIMER, navigate to timer, OK). If I navigate to either Name or Description, LEFT and RIGHT open the VirtualKeyBoard screen instead of moving the ConfigText's cursor right and left. I can't (for the moment) work out what's causing it.

TimerEntry overrides keyLeft & keyRight to have name & description use the virtual keyboard.

[prl]

They do fix the issue in the SMS-input information panel (NumericalTextInputHelpDialog) that IanSav raised in the comments in the issue tracker entry for Bug #317.

Ah, now I see what that is. I only anticipated single lines, I'll see if I can make it work with multiple.

As far as I can see, your changes to eTextPara and eLabel fix that problem completely. I don't think there's anything more to be done. The text in the NumericalTextInputHelpDialog entries is not multi-line, it wraps when there is no marked position, and is displayed non-wrapped when a marked position is set. That switch is made in eLabel::event(), in the evtPaint event handling.

[adoxa]

As far as I can see, your changes to eTextPara and eLabel fix that problem completely. I don't think there's anything more to be done.

The elongated cursor is weird, so making that the font/line height, not the widget height, might be nice.

The text in the NumericalTextInputHelpDialog entries is not multi-line, it wraps [...]

If it wraps it's multi-line.

[...] when there is no marked position, and is displayed non-wrapped when a marked position is set.

Oh yes, if there's a mark there's no wrapped flag. You're saying that's intended, rather than a bug? Given that alignCenter tests for it, I guess it is intended. It also ignores vertical alignment. Why should marked text be displayed differently from non-marked? It does complicate things, but I'll see what I can do. At the least I'll change the inverted height.

[prl]

Disappointingly, they don't fix similar problems in ConfigText widgets, because their representation is via eListboxPythonConfigContent, a subclass of eListboxPythonStringContent, rather than eLabel. I'd forgotten that

I think this might be better fixed by always using (a possibly calculated) visible_width (I tested with the EPG cache name, and that displayed right over top of the description). Is there another setting I should test?

They're two separate issues. One is that in a eListboxPythonStringContent, if there is a label text, the label text is simply left-aligned and the value text is right aligned and both are rendered with the whole width of the containing eListBox, with no protection from overlap. The other is that if the text in the value is longer than the width of the box, it doesn't scroll.

They should probably both be fixed, perhaps by restricting the value's width to the part of the row not used by the label (with some small amount of separation).

The "epg" text is the one I tested on, too.

[prl]

As far as I can see, your changes to eTextPara and eLabel fix that problem completely. I don't think there's anything more to be done.

The elongated cursor is weird, so making that the font/line height, not the widget height, might be nice.

The text in the NumericalTextInputHelpDialog entries is not multi-line, it wraps [...]

If it wraps it's multi-line.

I meant "not containing newlines".

[...] when there is no marked position, and is displayed non-wrapped when a marked position is set.

Oh yes, if there's a mark there's no wrapped flag. You're saying that's intended, rather than a bug? Given that alignCenter tests for it, I guess it is intended. It also ignores vertical alignment. Why should marked text be displayed differently from non-marked? It does complicate things, but I'll see what I can do. At the least I'll change the inverted height.

Yes, I'm pretty sure that's intended. If it wasn't done, editing texts that would otherwise wrap would be much more complicated.

I think the inverted height is a bug. It should be no more than the text height, or perhaps its line spacing.

[adoxa]

I wonder if the marked position could be passed along to gPainter::renderText/eTextPara::renderString? That would completely remove the whole m_pos != -1 block from eLabel::event, instead just adding m_pos to renderText. renderText would pass that along to renderString, which would then invert the glyph(s) directly, and to realign, to position it.

Yeah, maybe I'll just change the height.

[prl]

I think there's a lot to be said for simplifying that code in eLabel::event(). It always take me at least half an hour to work out what it's doing each time I need to look at it.

[prl]

I've also found a puzzling problem in 20180901 with the TimerEntry screen (from live TV, TIMER, navigate to timer, OK). If I navigate to either Name or Description, LEFT and RIGHT open the VirtualKeyBoard screen instead of moving the ConfigText's cursor right and left. I can't (for the moment) work out what's causing it.

TimerEntry overrides keyLeft & keyRight to have name & description use the virtual keyboard.

I was looking in the wrong places - at ActionMaps, not potential overrides. I'd never noticed that behaviour before.

IMO, that's a misfeature, and would be even more of a misfeature if scrolling in ConfigText and friends was implemented.

There are a number what I'd call misfeatures in TimerEntry for similar overrides: the waylaying of the functions of CH+/- and VOL+/- are the ones that I encounter most often, but there are others of what I think are of dubious value, like LEFT/RIGHT as a synonym of OK in the Channel entry (and in the Tags entry, if it were available).

It sounds very ripe for IanSav's Great UI Consistency Drive.

[IanSav]

Hi Prl,

I was looking in the wrong places - at ActionMaps, not potential overrides. I'd never noticed that behaviour before.

IMO, that's a misfeature, and would be even more of a misfeature if scrolling in ConfigText and friends was implemented.

There are a number what I'd call misfeatures in TimerEntry for similar overrides: the waylaying of the functions of CH+/- and VOL+/- are the ones that I encounter most often, but there are others of what I think are of dubious value, like LEFT/RIGHT as a synonym of OK in the Channel entry (and in the Tags entry, if it were available).

It sounds very ripe for IanSav's Great UI Consistency Drive.

I am still working on it. I have been bogged down in getting all the languages correct in VirtualKeyBoard and NumericalTextInput for OpenPLi and OpenViX. I think we are getting to the end of this process and then I will go back to the Setup / ConfigList / config refactor. (The latter actually used improvements in the former.) Hopefully there isn't much more coding to do but then comes the implementation and merging process. I thought VirtualKeyBoard and NumericalTextInput would be fast and easy to implement and merge. Unfortunately that has not proved to be the case.

The new button assignments seen in VirtualKeyBoard match those in the new Setup system. When that is complete I intend to update Menu and then follow through to the other common controls.

I am happy to keep providing code so you can monitor my changes but it would obviously be easier for everyone if the changes would also be merged into the Beyonwiz image.

Regards,
Ian.

[adoxa]

Here's the patch that fixes marked position/scrolling the "right" way. I might actually ask for comments before submitting, this time, though. In particular, it still keeps the cursor in the middle (my preference; also simplest to implement). Perhaps there should be an option for the cursor to move to the edge, then scroll the text (prl's preference). That also suggests a "hybrid" option, where the cursor moves to the edge, then both cursor and text scroll to the middle.

It fixes all known issues: scrolling in Input (InputBox and VirtualKeyBoard); making use of the second line in the SMS 0 help; and scrolling in ConfigText, as well as preventing overwrite of its label.

With proper scrolling (and ConfigText sizing) available that basically renders visible_width redundant. Once submitted and accepted I'll submit a patch to remove it from File Commander's line editor. Should other instances be removed? Timer's name and description, perhaps, although the description might look overlong.

TimerEntry overrides keyLeft & keyRight to have name & description use the virtual keyboard.

IMO, that's a misfeature, and would be even more of a misfeature if scrolling in ConfigText and friends was implemented.

Since TEXT can be used to bring up the keyboard, I've submitted a separate request to restore left & right.

[IanSav]

Hi Adoxa,

Do you have a T3 build of this patch?

Are you intending to make this change across all Enigma2 images? If not, then aspects of the change, even if an improvement, may not be wise. I am particularly looking at the removal of the visible_width parameter. Playing with NumericalTextInput (if that is your intention) could also have some ramifications.

Regards,
Ian.

[adoxa]

Do you have a T3 build of this patch?

No, but I could make one...

Are you intending to make this change across all Enigma2 images?

I only have a Beyonwiz, so no. And just what does "all" even mean?

I am particularly looking at the removal of the visible_width parameter.

I wasn't going to remove it, just reduce its use, since there won't be a need to use it for scrolling.

Playing with NumericalTextInput (if that is your intention) could also have some ramifications.

That's unchanged.

[adoxa]

Here's the T3 build of this patch (untested, hope it all works).

[IanSav]

Hi Adoxa,

By all images I believe that the code needs to be injected into OpenPLi and then OpenViX needs to be alerted to the appropriate commits to pull down and merge. This appears to then get the code into most of the other images.

Mind you I am still quite fuzzy on exactly how all the code gets around and how all the images interact. All my efforts are simply directed to OpenPLi and OpenViX. So far none of my changes has created any issues for which I am aware.

I have downloaded the patch and will try it out ASAP. Just working with OpenViX on something at the moment.

Regards,
Ian.

[prl]

Here's the patch that fixes marked position/scrolling the "right" way.

You may want to tidy this up with a suitable cast:

Code: Select all

../../easy-ui-4/lib/gdi/font.cpp:1227:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (markedpos >= glyphs.size())
         ~~~~~~~~~~^~~~~~~~~~~~~~~~

I know that some of the code is awash with warnings (especially the SWIG-generated code), but it makes warnings more useful if unnecessary warnings can be avoided.

[adoxa]

You may want to tidy this up with a suitable cast: [...] warning: comparison between signed and unsigned integer expressions [-Wsign-compare] [...]

I didn't bother with any of those, choosing to remove the warning in configure:

Code: Select all

  CFLAGS="$CFLAGS -Wno-sign-compare" \
  CXXFLAGS="$CXXFLAGS -fvisibility=hidden -Wno-sign-compare -Wno-narrowing -Wno-delete-non-virtual-dtor -Wno-deprecated-declarations -Wno-unused-variable"

That gives me a warning-free build, in conjunction with a script to remove SWIG and ar warnings:

make-T2-enigma2

Code: Select all

export SWIG_FEATURES=-w312,314,325,351,362,401,402,454,503,509
cd T2/lib && \
make ARFLAGS=cr actions/libenigma_actions.a base/libenigma_base.a \
	components/libenigma_components.a dvb/libenigma_dvb.a \
	dvb_ci/libenigma_dvb_ci.a gdi/libenigma_gdi.a \
	service/libenigma_service.a driver/libenigma_driver.a \
	nav/libenigma_nav.a gui/libenigma_gui.a mmi/libenigma_mmi.a \
	network/libenigma_network.a python/libenigma_python.a && \
cd ../main && make && $STRIP --strip-unneeded enigma2

[adoxa]

By all images I believe that the code needs to be injected into OpenPLi and then OpenViX needs to be alerted to the appropriate commits to pull down and merge. This appears to then get the code into most of the other images.

I see OpenPLi has the memory leak fixes, so it looks like they're monitoring Beyonwiz, anyway.

[IanSav]

Hi Adoxa,

By all images I believe that the code needs to be injected into OpenPLi and then OpenViX needs to be alerted to the appropriate commits to pull down and merge. This appears to then get the code into most of the other images.

I see OpenPLi has the memory leak fixes, so it looks like they're monitoring Beyonwiz, anyway.

That was me paddling as hard as I can to keep images in sync. Unfortunately none of the other images look to the Beyonwiz repositories for any changes. With Beyonwiz falling further and further behind it is getting harder to keep on top of these things. Small quick fixes like this are an exception.

In this case I notified OpenPLi and OpenViX with the commit pointers and asked Huevos if he could replicate the fix. It is in OpenPLi and will be in OpenViX shortly.

Regards,
Ian.

[prl]

You may want to tidy this up with a suitable cast: [...] warning: comparison between signed and unsigned integer expressions [-Wsign-compare] [...]

I didn't bother with any of those, choosing to remove the warning in configure:

Code: Select all

  CFLAGS="$CFLAGS -Wno-sign-compare" \
  CXXFLAGS="$CXXFLAGS -fvisibility=hidden -Wno-sign-compare -Wno-narrowing -Wno-delete-non-virtual-dtor -Wno-deprecated-declarations -Wno-unused-variable"

That gives me a warning-free build, in conjunction with a script to remove SWIG and ar warnings:

Perhaps, but it really should be fixed in the code, as

Code: Select all

     if (markedpos >= (int)glyphs.size())

noting that

Code: Select all

     if ((size_type)markedpos >= glyphs.size())

won't test correctly for allowed values of marked_pos.

I really don't think it's a good idea to "remove" legitimate warnings by having the compiler ignore them.