Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#3395 closed Enhancement (fixed)

Open files by double-clicking on them in the files list

Reported by: sng Owned by: charles
Priority: Normal Milestone: 2.10
Component: GTK+ Client Version: 2.01
Severity: Normal Keywords:
Cc:

Description

One thing I've been missing is the ability to open the downloaded files from the file's list, instead of opening the containing folder...

So, this is a little patch I made, in order to be able to double click in the file's list and have gnome open the file for me.

Notes:

  • gnome-open is used to open the files
  • filename length is set to 1024. Don't know a way to get a gtk/gnome/ whatever limit

Attachments (4)

shell_execute.2.diff (2.6 KB) - added by sng 12 years ago.
shell_execute.diff (4.4 KB) - added by sng 12 years ago.
latest update, adding new function
shell_execute.r4.diff (2.9 KB) - added by charles 12 years ago.
r4: drop the use of FC_INDEX and FC_PROG. Handle the case where a file's folder doesn't exist yet either.
shell_execute.r5.diff (2.7 KB) - added by charles 12 years ago.
cleaner version of buildFilename()

Download all attachments as: .zip

Change History (26)

comment:1 Changed 12 years ago by charles

  • Version changed from 2.01+ to 2.01

comment:2 Changed 12 years ago by charles

sng:

  • I think you could / should use the "row-activated" signal instead of rolling your own doubleclick detection
  • You should use gtr_open_file() instead of the system() command

Changed 12 years ago by sng

comment:3 Changed 12 years ago by sng

charles:

ok, both your suggestions are now satisfied :)

comment:4 Changed 12 years ago by sng

Hi again

This is the final (?) version of the patch

When requesting the opening of a non-complete file, the containing folder is opened instead

comment:5 Changed 12 years ago by howl

Perhaps "const char * tr_torrentGetCurrentDir( const tr_torrent * tor );" or "char* tr_torrentFindFile( const tr_torrent * tor, tr_file_index_t fileNo );" in libtransmission/transmission.h could be used for this patch to avoid doing the code to get the files location and reuse the existing code.

I like this ticket, open files directly from the interface will be useful.

comment:6 Changed 12 years ago by charles

Howl is right -- you could pull FC_INDEX and pass the value along to tr_torrentFindFile(). That way, I don't think you'd have to iterate up the model at all.

Also I'm not sure what "have", "size", "enabled", and "priority" are used for here. It looks like they're read in from the model but never used.

Third, it's probably a little better to use g_path_get_dirname(), or G_DIR_SEPARATOR, instead of strrchr(str,'/') for portability's sake.

comment:7 Changed 12 years ago by sng

Here I am again

I'm afraid iterating up the model cannot be avoided (unless there's a tr_torrentFindFolder that I cannot find). When i double click a folder, i get invalid FC_INDEX

comment:8 Changed 12 years ago by sng

Another update...

Using FC_INDEX to get a valid filename, introduced two new problems

a) FC_INDEX only works for files, so when the user double-clicks a
   folder, no filename is returned by tr_torrentFindFile (actually
   no index is return, so no filename can be retrieved)
b) FC_INDEX returns a valid index for files that have already been
   created on disk. Apart from the problem of not being able to
   handle such a file (to open its containing folder, that is), an
   inconsistency appears to the user. 

   When a files is reported to have 0% progress (on gui), this means:
   1. the file exists and its progress is 0-1%. This file can be
      handled using its index
   2. the file does not exists yet (0% progress). This file cannot be
      handled using its index

In order to fix this problem we have to iterate up the model (which
is what we wanted to avoid when we used FC_INDEX in the first place).
So I have written a function that does just that. I called it
torrentFindFolder but this may not be the right name for it (I just
did not know what to call it :) ) It actually uses the iterator to
return a filename or a pathname (or NULL in case of failure)

Changed 12 years ago by sng

latest update, adding new function

comment:9 Changed 12 years ago by charles

Looking over r3, I think you're right. It's less code to use gtk_tree_model_iter_parent(), since we'd have to use it as a fallback if FC_INDEX fails anyway. And if we ignore FC_INDEX we don't have to worry about FC_PROG either.

r3 doesn't handle the case when the parent directory doesn't exist yet, either. This can happen in nested hierarchies in a .torrent file.

Attached is a r4 version of the diff. Please feel free to revise it or give feedback on it.

Changed 12 years ago by charles

r4: drop the use of FC_INDEX and FC_PROG. Handle the case where a file's folder doesn't exist yet either.

comment:10 Changed 12 years ago by charles

  • Summary changed from Open files (using the default app) by double-clicking on the files list to Open files by double-clicking on them in the files list

Changed 12 years ago by charles

cleaner version of buildFilename()

comment:11 Changed 12 years ago by charles

sng: what do you think about this patch? Do you have any suggestions or changes you'd like to make before I commit it?

comment:12 Changed 12 years ago by sng

hi Charles sorry, i was off for a while

I'm afraid you still have to check FC_PROG when g_file_test( filename, G_FILE_TEST_EXISTS ) returns true, cause you don't want to try to open an incomplete file (could happen when ".part" extension is not used for incomplete files)

comment:13 Changed 12 years ago by charles

  • Milestone changed from None Set to 2.10
  • Status changed from new to assigned

Thanks for that!

Added to trunk by r10995

comment:14 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:15 Changed 12 years ago by vec

Hi, I am new here and I really do not understand much of it, but I write, because I desired that patch, too and have a (minor) issue with it. When you have a file and it does not exist, from what I did understand, the parent folder will pop up. I can see no good reason to do so. For big archives this could cause a a lag, even. On the other hand opening a file not completely loaded e.g. a videofile is not very usefull, because of heavy fragmentation of incomplete downloads. To sum it up, when I really want to go to the folder, i should just use the context menu. anyway, I'm glad someone else did this before i had the chance to screw it up.

Last edited 12 years ago by vec (previous) (diff)

comment:16 Changed 12 years ago by sng

Hi vec,

the truth is this point troubled me too at first. Let's see the different options:

  1. Double click on a folder - just open it
  1. Double click on a file
    1. if it's fully downloaded, open it
    2. if it's not
      1. open it's parent folder (current implementation)
      2. do nothing (I don't like this option, it's like the program does not work well...)
      3. ask the user what to do
      4. display a message box stating that the file cannot be opened
      5. have a configuration option about this situation (would this be possible, Charles?)

As you see things could get quite complicated, in order to have such a small gain (just open a file). This is why I preferred (and still do) to just open the parent folder

But I suppose it's up to Charles from now on :)

Last edited 12 years ago by sng (previous) (diff)

comment:17 Changed 12 years ago by vec

Telling from my previous use of µTorrent and Azureus I would expect an unfinished file to get loaded, unless it does not exist. That is barely usefull for images or movies, if you get lucky, but it contradicts with my idea of avoiding unnessecary loads. A messagebox asking to open the file together with a checkbox "open the file by defautl and do not ask again" + config option, would almost be overkill, but a feasable solution -- in my opinion. Opening a file complete or incomplete, as long as it exists seems to be the easiest solution. I did not intend to argue with you, tho. Just giving a heads-up.

comment:18 Changed 12 years ago by sng

hi again vec

Thre's no problem about expressing your opinion; we're just talking and trying to make it better for all of us. I do not think my last message was offensive or aggressive in any way, but if it seemed like it, I'm really sorry :)

As I said before, it's just a matter of opinion, so i think it's up to Charles to decide

comment:19 Changed 12 years ago by charles

I think sng is right about the options we have:

  1. open its parent folder
  2. do nothing
  3. ask the user what to do
  4. display a message box stating that the file cannot be opened
  5. have a configuration option about this situation

I don't like 2 or 5. 2 seems as if the feature is broken, and punting behavior choices to preferences (as in 5) is always bad.

4 is a little better than 2, since it gives some user feedback.

3 is a little better than 4, since it gives the user more choices.

IMO 1 is the best general choice. I agree it could be slow if the directory is large, but think that's an acceptable tradeoff for not having to throw another dialog in the user's way the rest of the time.

comment:20 Changed 11 years ago by sng

Hi again Charles

I see 2.02 and 2.03 have been released but this feature has not been included yet :(

Is there something else that has to be done in order to see it in 2.04? Is there a way to speed things up?

Thanks, Spiros

comment:21 Changed 11 years ago by Longinus00

sng,

The 2.0x releases are bug fixing releases, new features will will appear in version 2.10. Think of the first decimal place as a minor version number, e.g. 2.0.3 and 2.0.4 vs. 2.1.0.

comment:22 Changed 11 years ago by sng

Thanks for the answer Longinus00

I'll just have to wait for 2.10 then

Note: See TracTickets for help on using tickets.