Under certain conditions, it was possible to get orderLinks() trigger a
segmentation fault or a SIGBUS; even if all the Link* and Linkapp*
pointers where correct, the compare_links() function would eventually
end up receiving an invalid pointer.
Apparently, the std::sort function *requires* that the assertion
compare_links(a, b) != compare_links(b, a) is always true, which was not
the case previously when a link was compared with itself.
It is possible for OPK links to refer to a missing icon or to an icon
that cannot be read as a PNG file, in which cases there is no fallback
to icons/generic.png and the icon is null.
Link::paint checks for this, but LinkApp::drawLaunch did not.
Obviously it is more desirable to properly fall back, but that would be
a very big change to many areas of the code, so this is a quick fix.
Instead, consider the installation location non-deletable and the home
directory deletable. This makes the "deletable" property follow the
purpose of the directory rather than its implementation details.
There are various situations in which write() might write less than
the requested amount. In that case, if any progress was made (more
than 0 bytes written), try again, otherwise consider it an error.
Thanks to Nebuleon for reviewing.
The check I removed earlier today was redundant for readPackages() but
not so much for the Monitor: it starts a thread and it logs an error,
both of which are better avoided when possible.
Also cleaned up implementation of readPackages() a bit.
This is safer than the original code, which didn't use a temporary file
and could therefore leave partial files. Also it avoids a full sync(),
which can take a long time if for example a big file transfer is going
on or recently ended.
Since only non-default values are saved, it was quite likely settings
files were empty, especially since the clock saving was fixed. We now
remove a settings file on save if the contents are empty, so we have
to load fewer files on startup.
This avoids having to store the paths in an intermediate vector. Also
it allows us to do the writability check once per directory instead of
once per file.
Previously, it sorted before Link instances were created. Using
readLinks() ensures consistency and it might be more efficient if
a significant number of link files are rejected.
Also modernized the body of orderLinks() and removed unnecessary
special case for 1-link sections.
For zero keys, the OPK file itself would be parsed as a settings file.
For multiple keys, all first categories would be appended to the path,
leading to a much too deep directory.
Before this change, it was possible to delete for example a link that
is stored on a read-only file system and it would disappear after
deletion but reappear as soon as the menu restarts.
In Linux "/.." exists and loops back onto the root. But for the user
it is confusing to see a ".." entry listed that does nothing when
selected, so hide it.
This can happen on a directory with 0 file matches when directories are
not being shown (otherwise there is either ".." or a subdir).
Since there is nothing to be selected, the Accept button is disabled
and no cursor is shown.
Renamed fontHeight to lineHeight, since it is computed using both the
font's line spacing and the height of the folder icon. For the latter,
use the actual icon height plus two pixels spacing instead of the
hardcoded value of 20 (16 height + 4 pixels spacing).
Fixed recently introduced bug where "top" is added to the folder icon
y-coordinate twice. Also center the icon vertically when a large font
is used.
Don't crash if the folder icon is missing. This could only happen on
broken installations though. Also don't search for the icon twice:
SurfaceCollection::skinRes already calls addSkinRes internally when
there is no cached surface for the key.
Removed unused data member selRow.
Keep reference to LinkApp instead of pointer.
Call LinkApp::getSelectorBrowser once and store result.
Renamed fontheight to fontHeight.
Use iY consistently: store the item's y-coordinate in it.
This removes the need for a separate setAction method.
The default action is the empty action, which does nothing. However,
a touch event on a button with the empty action is no longer considered
handled.
Menu::btnContextMenu was changed from a unique_ptr to a plain data
member.
It was only used to fetch resY, so I replaced that by taking the
y-coordinate as an argument. That is also more consistent with the
x-coordinate which was already an argument.
The x-coordinate was changed to a signed int, since that is the norm
for paint coordinates. It can be useful for drawing widgets that are
partially off screen, for example during a (dis)appear animation.
In InputDialog, the ButtonBox field was changed from a pointer to
a plain data member. There was no need to dynamically allocate it.
Previously, IconButton instances to be added to button boxes were
allocated with new, but never freed with delete.
unique_ptr makes sure the buttons will be freed along with the button
box that owns them, or when code calls ButtonBox::clear.
The destructor of ButtonBox has been made redundant by this change, so
it's gone.
Put the drawButton calls left-to-right.
Avoid duplicate code.
The order was changed: "Select" is now always first, to be consistent
with other dialogs.
Instead of correcting the returned coordinate with "- 10" externally,
omit the white space inside the methods.
Note that Font::getTextWidth, which was used until recently, considers
an empty string to have width 1, so 3 + getTextWidth("") + 6 == 10.
There is a difference in how buttons that have neither a label nor an
icon are positioned in the new code, but that is a situation that
should not occur in practice. Plus I'd argue that the new behavior is
actually better in that case.
This was only called form ButtonBox, so I moved the code there.
I still think a paint method shouldn't be repositioning widgets, but
that's something for a later cleanup.
