New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use prettyname_regex to show shortname in update menu #39

Merged
merged 1 commit into from Nov 17, 2016

Conversation

Projects
None yet
2 participants
@lrusak
Member

lrusak commented Nov 8, 2016

I'm not 100% happy about this but it does work. It does it in a workaround way though

Instead of tracking the object all the way through it first uses the regex on the filenames to get the short filenames, then when one of the short filenames is chosen it takes that short name and scans the list of long filenames to find a match, which may not be 100%. We have set filenames though and it shouldn't be a problem I don't think.

This was the only way I could think of doing it because self.struct['update']['settings']['Build']['values'] needs to be a list, so I cannot pass a dict to it to track a prettyname and a filename for one object.

@MilhouseVH please include this in your testbuilds, it will work for you already because you include prettyname_regex already in your releases.json

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Nov 8, 2016

Contributor

Thanks very much for this, it seems to be working!

Is there anything that can be done when showing the current version on the confirmation dialog?

Currently it shows the following:
s1

when the following would be better:

Current Version: #1108
New Version: #1107
Would you like to update LibreELEC now?

         [Yes]   [No]
Contributor

MilhouseVH commented Nov 8, 2016

Thanks very much for this, it seems to be working!

Is there anything that can be done when showing the current version on the confirmation dialog?

Currently it shows the following:
s1

when the following would be better:

Current Version: #1108
New Version: #1107
Would you like to update LibreELEC now?

         [Yes]   [No]
@lrusak

This comment has been minimized.

Show comment
Hide comment
@lrusak

lrusak Nov 8, 2016

Member

@MilhouseVH so the problem is that there is no way to store a regex on the system for the current version (at least not in a nice way). So there is no way to parse accurately what build the people are on currently, only what they will be updating to.

What I could do is use the current regex on the current version and if it returns something then use that otherwise just return the entire version string.

How does that sound?

Member

lrusak commented Nov 8, 2016

@MilhouseVH so the problem is that there is no way to store a regex on the system for the current version (at least not in a nice way). So there is no way to parse accurately what build the people are on currently, only what they will be updating to.

What I could do is use the current regex on the current version and if it returns something then use that otherwise just return the entire version string.

How does that sound?

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Nov 8, 2016

Contributor

Ah yes, that's true, good point.

Your suggestion would work for me. I could make the regex I use more specific if it returns false matches.

Edit: I've made my pretty_regex less likely to result in a false match - added Milhouse-.* to the match.

Contributor

MilhouseVH commented Nov 8, 2016

Ah yes, that's true, good point.

Your suggestion would work for me. I could make the regex I use more specific if it returns false matches.

Edit: I've made my pretty_regex less likely to result in a false match - added Milhouse-.* to the match.

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Nov 17, 2016

Contributor

I haven't had any negative feedback - good to merge?

Contributor

MilhouseVH commented Nov 17, 2016

I haven't had any negative feedback - good to merge?

@lrusak

This comment has been minimized.

Show comment
Hide comment
@lrusak

lrusak Nov 17, 2016

Member

I squashed it, if you're happy I'm happy :)

Member

lrusak commented Nov 17, 2016

I squashed it, if you're happy I'm happy :)

@MilhouseVH MilhouseVH merged commit 52ed9a5 into LibreELEC:master Nov 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment