Skip to content
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

[video] Readd mark watched / unwatched to video folder's context menu #12464

Merged
merged 1 commit into from Jul 25, 2017

Conversation

@ksooo
Copy link
Member

commented Jul 11, 2017

This is my desperate ( ;-) ) attempt to fix the problem reported here: https://forum.kodi.tv/showthread.php?tid=311447&page=2

It works, but I'm not sure whether the approach to use an item property to transport the type of the folder is okay. Feedback from devs is welcome.

This has been tested on latest master on macOS.

@notspiff fyi

@ksooo ksooo added this to the L 18.0-alpha1 milestone Jul 11, 2017

@ksooo ksooo requested a review from Montellese Jul 11, 2017

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2017

To whom it may concern: If nobody speaks up I will merge this PR the next days. ;-)

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2017

jenkins build this please

@@ -58,6 +58,8 @@ bool CMarkWatched::IsVisible(const CFileItem& item) const
{
if (item.HasVideoInfoTag())
return item.IsVideoDb();
else if (item.GetProperty("IsVideoFolder").asBoolean())

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jul 19, 2017

Member

If this can only be a recording folder or a video item, why not change the if-else to check video info tag, is recording, not parent folder and else return false.

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jul 19, 2017

Member

And please adjust the comment in line 57

This comment has been minimized.

Copy link
@ksooo

ksooo Jul 19, 2017

Author Member

Not sure I understand what you mean. The problem is that item can be anything (!) and we must only return true in case of certain items - namely videos/recordings and video folders/recording folders.

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jul 19, 2017

Member

I thought that this context class is only involved if the item holds a video item, because it's video/ContextMenus.cpp. If that is not the case ignore my comment.

@@ -82,6 +84,8 @@ bool CMarkUnWatched::IsVisible(const CFileItem& item) const
{
if (item.HasVideoInfoTag())
return item.IsVideoDb();
else if (item.GetProperty("IsVideoFolder").asBoolean())

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jul 19, 2017

Member

Same here + comment in line 83

This comment has been minimized.

Copy link
@ksooo

ksooo Jul 19, 2017

Author Member

See my comment above. Your suggestion does not work.

@ksooo ksooo force-pushed the ksooo:video-fix-folder-mark-watched-unwatched branch from 681546a to 286a0e0 Jul 24, 2017

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2017

Updated the comments, as requested.

I will merge this PR tomorrow, if nobody abjects.

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2017

jenkins error is unrelated

@ksooo ksooo merged commit 0f77778 into xbmc:master Jul 25, 2017

1 check failed

default Sorry, building this PR failed. Please check the logs for the errors.
Details

@ksooo ksooo deleted the ksooo:video-fix-folder-mark-watched-unwatched branch Jul 25, 2017

@Nostalgist

This comment has been minimized.

Copy link

commented Jan 4, 2018

Thanks for looking into this! Does this fix work for anyone? I can neither reset the watched status of files inside a folder via context menu nor by pressing "w".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.