The 0.9 branch doesn't delete (or at least shouldn't) songs on unavailable mount
points immediatly, but hides them. When the mount point reappears, so does the
songs. It doesn't let you specify a new mount point for the songs thoguh.

Is it possible to change this behaviour so that the songs are still visable,
with perhaps a warning icon next to them (this is the way iTunes does it)?

I think many users will be utterly confused by this behaviour. 

Dunno what is more confusing between hiding the songs when they aren't there and
having the user wonder where his songs are, and displaying the song and having
the user wonder why the songs are displayed and not playable.
Anyway, possibility #1 is implemented while #2 isn't, and there are more urgent
things to fix atm...

While I agree that this is certainly an improvement, I think you should look
towards implementing #2. If they try and play a song from the 'greyed out'
selection, then it should pop up with a warning saying 'This song cannot be
played because the share $sharename is not available. Please reconnect to the
network share $shareurl'. This would allow users to see exactly where the music
is mounted and would provide an immediate instruction on how to fix the error.

The code was written with nfs mounted volume and removable hard disks in mind, I
don't think an error message talking about mount point and stuff like that would
be understandable to mere mortals...
The current 0.9 code is supposed to work with network shares but hasn't been
tested with those, dunno if it really works.

If you have a mounted share but have not yet authenticated with your keychain
password in this session, Rhythmbox seems to also remove (hide?) the songs that
are on that share. 

It would be excellent if Rhythmbox could just prompt the user for the keychain
password in this case.

Bug 140355 is that Rhythmbox doesn't support displaying an authentication dialog
for gnome-vfs, which is why it can't see shares that have not been mounted (it
should work is they do not require user intevention, e.g. public key ssh mounts
when using ssh-agent). The last suggestion can't be done until that gets fixed.

Peronally I'd say that showing tracks that can't be played is worse than not
showing them, because it would be hard to give a meaningful error message to the
user. (just my 2c though)

Maybe a show-only-once info window could pop up at RB startup and tell that 
"songs from removable media or network shares are hidden. They will be as long 
as the media are not available".

That should avoid confusion. Do you agree, Martin?

*** Bug 315204 has been marked as a duplicate of this bug. ***

Since 0.9 doesn't lose tracks that are missing (it just hides them), I'm
retitling the bug to be the suggestion that it should have some indication that
tracks are hidden, or have a way of accessing the list of hidden tracks.

Created attachment 58855 [details] [review]
mostly done patch

This patch adds a new child of the library source that displays all files missing from the library.  It's only visible when there are one or more hidden entries in the library.

Other changes:
- add 'last seen' string, only stored for hidden entries
- add a few missing rhythmdb_commit calls after visibility changes
- adds a way to make any standard entry view column always visible (otherwise the last seen and location columns would disappear from the missing files source)
- adds the possibility of 'last seen' and 'location' columns
- corrects the name of rhythmdb_entry_move_to_trash_set_error

Since we can't stop RBShellPlayer from handling entry view row activation, it'll try to play entries we know are missing, which kind of sucks.

Created attachment 59050 [details] [review]
updated patch

Updated to current cvs.

I've also made it no try to play the tracks, by making RBShellPlayer not respond to track activations for hidden tracks.

One potential improvement would be to store the "trashed file location" for trashed files. That was the now playing column could be replaced with one that displays a "missing" or "trashed" icon depending on which is the case. It would also allow a "restore from trash" option.

Created attachment 59294 [details] [review]
updated again

I plan to add some way of dealing with removed and trashed files to this, but I'm not sure what exactly.  The next thing I'm going to add will be another meta-source for files that could not be imported.

This is *very* handy!  Patch works for me.  Would be great to get in soon to allow for testing.  

This is very handy for tracking down botched imports and duplicates that occasionally flub up the database, or for removing entries when the files have been moved around.  I had a bunch of renamed directories and this found some duplicates were hanging around and making life difficult when attempting to re-import songs when plugins were updated.  This is, I suspect, probably what causes bug #329985 which I was able to recently duplicate. 

My only suggestion might be to include the number of tracks as "34 missing tracks" in the status bar.  Currently it simply displays "0 songs".

Looks fine to me, unless you wanted to add some of the other bits before committing.

Updated patch status as per comment #15.

(In reply to comment #13)
> Created an attachment (id=59294) [edit]
> updated again

Patch doesn't apply cleanly against CVS:

patching file sources/rb-library-source.c
Hunk #1 FAILED at 420.
1 out of 1 hunk FAILED -- saving rejects to file sources/rb-library-source.c.rej

This is a very useful feature and seems to work OK.  Can this be committed soon?

Created attachment 59687 [details] [review]
now with 'import errors' source

This adds another meta-source for showing import errors, replacing the load failure dialog box (fixing bug #142322).

The other meta-source I'm thinking of adding is for removed/trashed files, but this requires a bit more thought, so I'll do it separately.

Still doesn't apply cleanly against CVS HEAD:

patching file sources/rb-library-source.c
Hunk #1 FAILED at 420.
1 out of 1 hunk FAILED -- saving rejects to file sources/rb-library-source.c.rejpatching file sources/rb-missing-files-source.c
patching file sources/rb-missing-files-source.h
patching file sources/rb-playlist-source.c
Hunk #1 FAILED at 252.
1 out of 1 hunk FAILED -- saving rejects to file sources/rb-playlist-source.c.rej

Created attachment 59688 [details] [review]
a bit more

I forgot to fix the status text for the meta-sources.

(In reply to comment #19)
> Still doesn't apply cleanly against CVS HEAD:

Sorry, ignore that last comment, was trying the wrong patch. 

Compile error with applied patch:

cc1: warnings being treated as errors
rb-missing-files-source.c: In function 'impl_get_status':
rb-missing-files-source.c:346: warning: passing argument 1 of 'gtk_tree_model_iter_n_children' from incompatible pointer type
make: *** [rb-missing-files-source.lo] Error 1

Created attachment 59690 [details] [review]
fix compile warnings

With this patch, trashed sources would show up under the "missing" source, since they are just hidden in case the user wants to move them back. We could potentially store the "deleted location" somewhere, so that we can tell the difference, and offer an un-trash operation.

Appears that can be fixed by using the GTK_TREE_MODEL macro:

count = gtk_tree_model_iter_n_children (source->priv->model, NULL);

to:

count = gtk_tree_model_iter_n_children (GTK_TREE_MODEL (source->priv->model), NULL);

in two files.

With new patch, all is working well.  I tested both the hidden/missing files child source and imported a folder with non-music files and tested the import errors child source.  (Obsolete "bit more" patch).

(In reply to comment #23)
> Created an attachment (id=59690) [edit]
> fix compile warnings

Again, failed to apply cleanly against current CVS.  Sorry to harp on about this, but is there any reason *not* to commit this to CVS? 
patching file shell/rb-shell.c
Hunk #3 FAILED at 136.
Hunk #4 succeeded at 373 (offset 3 lines).
Hunk #6 succeeded at 1178 (offset 3 lines).
Hunk #8 succeeded at 1207 (offset 3 lines).
Hunk #9 succeeded at 1292 with fuzz 2.
Hunk #10 succeeded at 1570 (offset 4 lines).
Hunk #12 succeeded at 1660 (offset 4 lines).
Hunk #13 succeeded at 1735 (offset 2 lines).
Hunk #14 succeeded at 2132 (offset 12 lines).
1 out of 14 hunks FAILED -- saving rejects to file shell/rb-shell.c.rej

It looks fine to me, unless we wanted to add a "trashed files" source, or make the missing files source not the difference between missing and trashed ones (possibly with undelete).

OK, committed.

Um, build fails in CVS after this checkin.  Did a "make distclean" and re-ran autogen.sh etc:

-I../daapsharing -I/usr/include/libsoup-2.2 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -g -O2 -Wcomment -Wformat -Wnonnull -Wimplicit-int -Wimplicit -Wmain -Wmissing-braces -Wparentheses -Wsequence-point -Wreturn-type -Wswitch -Wtrigraphs -Wunused-function -Wunused-label -Wunused-value -Wchar-subscripts -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wcast-align -Wall -Werror -std=gnu89 -MT rb-shell.lo -MD -MP -MF .deps/rb-shell.Tpo -c rb-shell.c  -fPIC -DPIC -o .libs/rb-shell.o
cc1: warnings being treated as errors
rb-shell.c:147: warning: 'rb_shell_load_failure_dialog_response_cb' declared 'static' but never defined
make[3]: *** [rb-shell.lo] Error 1
make[3]: Leaving directory `/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell'make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell'make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/alex/src/remote-cvs/gnome.org/rhythmbox'
make: *** [all] Error 2

Fixed again