I haven't actually tested this yet, but I suspect this is another case of bug #355047, article download (to cache) becomes save after restart, which was fixed a couple months ago, and that it's not specifically $HOME, but rather $PWD, which is normally $HOME unless someone is starting pan from a shell where they've switched to a different dir.

Or more technically, I think #355047 was a subset bug of this one, only it was reported first and the fix for it, retaining the cache vs. save info, didn't fix the other half, that it was saving to $PWD instead of the default save dir, so that half is showing up again here.  Maybe the work from it will make fixing this one a bit easier.

Alternatively... this could be declared a feature not a bug, as when the *.nzb is fed to pan on the command line, it uses the working dir rather than the configured default dir as the save location.  However, if that is to be the case, then a parallel command line option to change the save location from the default $PWD would be useful as well.  That could be implemented either as an option to have pan change to that working dir before doing anything else, or as a command-line save location, possibly taking precedence over any configured default.

Good catch Duncan!  That was it, so in the meantime I can just use a script to launch Pan to avoid this problem.

I think it is probably a common use case for people to open a nzb in pan from their browser rather than save it and then open it with Pan so I think it might be better from a user perspective to behave the same way when a nzb is imported from a command line as it does when imported from inside the app and ask the user where they want to save the attachments.

While I would like to see this I can understand if it needs to slip to 1.1 due to time constraints, especially since there is an easy workaround.

Just some follow-up to this, Duncan here is the proof that you are 100% correct:
from pan.cc ~line 194

  char * pch = g_get_current_dir ();
  std::string nzb_output_path = pch;
  g_free (pch);


So now let me see if I can hack in some way of opening the standard gtk save dialog box to populate that variable rather than just using the current path.

Created attachment 79130 [details] [review]
Patch to add a save dialog when a nzb is passed from the command line

Ok this is a hack, I took the save dialog from gui.cc and hacked it in so that when pan.cc calls for the save path instead of it returning pwd it opens the save dialog and returns that path.

Rather than add some additional code to import the preferences I had it return the current path as the starting point for the save.  I might take a look at adding that back in once I figure out how  ;-)   

I am sure I am making a ton of mistakes and that Charles will find it easier to just write this himself rather than fix my mistakes but it gave me some experience in how the dialogs work in gtk.  Too bad I will forget it all due to the champagne tonight!

Created attachment 79131 [details] [review]
Attachment to open Save dialog when nzb added via command line

Oops, I had some whitespace that got carried over into the diff.

Created attachment 79132 [details] [review]
Add save dialog when nzb is opened from a command line

I did something stupid, pch isn't needed anymore since I can just use nzb_output_path directly.

Created attachment 79134 [details] [review]
Patch to add save dialog when a NZB is added via command line

*smacks head*  Ok my previous patches caused pan to prompt for a save location when it started rather than just when a nzb was passed via command line.   I changed my patch to only call the save dialog when a nzb was passed to it.

Ok now time to go celebrate the New Year!

does this patch pop up the save dialog even if --nogui is passed in?

I am not sure, I will test it tonight and find out.  If it does I will fix it.

Yes, it looks like it will pop-up even if --nogui is passed since the check for no gui occurs after the nzb's are loaded..  I am not quite sure how to handle it except to break --no-gui into a new function.  Would that clutter it up or do you have any suggestions?

Well I'm assuming this patch is for the benefit of URL handlers, since someone
just running Pan from the command line can use -o or --output= to specify
where the files get saved. 

So I guess in the case where Pan is being used as a URL handler, the dialog
makes sense.

Correct that is exactly what I do and why I wrote the patch.  However even if they specify -o the prompt still comes out so I need to fix that.

How about this (I haven't actually tried this but so this isn't a real patch just me playing around in gedit)
     // load the nzb files...
-     std::vector<Task*> tasks;
-     foreach_const (strings_t, nzb_files, it)
-       NZB :: tasks_from_nzb_file (*it,get_save_attachments_path_from_user (), cache, data, data, data,tasks);
-     queue.add_tasks (tasks, Queue::BOTTOM);

     // don't open the full-blown Pan, just act as a nzb client,
     // with a gui or without.
     if (gui) {
+       std::vector<Task*> tasks;
+       foreach_const (strings_t, nzb_files, it)
+          NZB :: tasks_from_nzb_file (*it,get_save_attachments_path_from_user (), cache, data, data, data,tasks);
+       queue.add_tasks (tasks, Queue::BOTTOM);
       TaskPane * pane = new TaskPane (queue, prefs);
       GtkWidget * w (pane->root());
       gtk_widget_show_all (w);
       g_signal_connect (w, "destroy", G_CALLBACK(destroy_cb), 0);
       g_signal_connect (G_OBJECT(w), "delete-event",G_CALLBACK(delete_event_cb), 0);
     } else {
+     std::vector<Task*> tasks;
+     foreach_const (strings_t, nzb_files, it)
+       NZB :: tasks_from_nzb_file (*it,nzb_output_path, cache, data, data, data,tasks);
+     queue.add_tasks (tasks, Queue::BOTTOM);
+       nongui_gmainloop = g_main_loop_new (NULL, false);
     }
     register_shutdown_signals ();
     mainloop ();

That muffed up the formatting but the basic idea is that rather than load the nzb's before we go into the decision for if we load the GUI or not we load it after that decision is made.

It doesn't look very elegant to me since we are replicating code but I /think/ it would work.  I won't be able to try it until tonight.  

Thoughts?

Created attachment 79239 [details] [review]
Try #5 for save attachment patch

Ok this is much better, after we check if nzb is true I then do a check if gui is true and at that point set nzb_output_path = the save function and then return back as normal.  The only difference is that now nzb_output_path has the users preferred save path rather than pwd.

How does this look?  Any chance it can make it in before 1.0?  With a lot of people using NZB files from websites I think there are a large number of users who would appreciate this functionality.

Thanks!

Created attachment 79397 [details] [review]
revision of previous patch.

Here's another try.  It's basically the same as #5, but:

* always honors the -o argument, if specified

* falls back to user's home dir, rather than current dir,
  which makes more sense when Pan's being invoked by another app

* makes the gui.cc dialog function public, so that pan.cc can
  call it instead of duplicating the code

What do you think?

I think it looks great!  One question though, so if the user hits cancel and no path is returned it will just save to the users preferred path?

Created attachment 79401 [details] [review]
Try #7

The previous patch wouldn't work if the user pressed cancel,
but in this version it will fall back to the user's
home directory.

Oh I see, there will be a new preferences option to prompt for save path.  If
that is not set and the user did not use the -o option then $home will be used.

I think that looks perfect though to avoid usability issues I would say that
the default should probably be to prompt.

On another note if the user does have it set to prompt and then hits cancel shouldn't that cancel the entire process and not just fallback to $home?   

(In reply to comment #15)
> Created an attachment (id=79397) [edit]
> revision of previous patch.
> 
> Here's another try.  It's basically the same as #5, but:
> 
> * always honors the -o argument, if specified
> 
> * falls back to user's home dir, rather than current dir,
>   which makes more sense when Pan's being invoked by another app
> 
> * makes the gui.cc dialog function public, so that pan.cc can
>   call it instead of duplicating the code
> 
> What do you think?

I think I don't want additional files dumped directly in my home dir under any conditions, unless I specifically dump them there (and defaulting to it doesn't count as specifically dumping them there!).  I already have enough stuff there to worry about and don't need anything more to need to sort thru.  There's a reason my current pan starter scripts cd to a ~/pan/scraps/ subdir, since pan did at one point without warning start dumping stuff in its current working dir, which by default is $HOME.

However, if I get this code correct, I can simply test to see if -o is passed to my script on the commandline, and if not, add "-o ~/pan/scraps" to the line, just in case, in addition to cding to ~/pan/scraps before I start pan, and that should keep stuff out of $HOME.  I don't particularly like it, preferring $PWD as the default if there's no configured default, but as long as I can set -o unconditionally in the script (it won't do weird stuff if pan isn't invoked with a *.nzb, right?), I don't suppose I can protest /too/ loudly. =8^)

Duncan (who's a bit grouchy today due to nothing pan related, hope it doesn't come thru /too/ badly)

The problem with using $PWD as the default is: what is $PWD when pan's
invoked from another application?  Is it the directory where pan or the
other application reside?  Does it vary by circumstances?  This is likely
to confuse users.

Yes, your script can check for -o and add it as necessary.

In my opinion if the user doesn't pass -o then if cancel is clicked it should cancel the whole operation.  It feels more natural for a user if the cancel dialog being clicked actually cancels the download.

Either way your patch works for my use case which is all that matters to me  ;)

Charles, out of curiosity will this make 1.0 or is it a 1.1 item?

Thanks!

It's not too large, and doesn't break the string freeze... IMO 1.0

Created attachment 80250 [details] [review]
Try #8 -- same as #7, but exit if user hits `cancel' in NZB dialog

Also since try #8 prompts whenever no path is specified,
and exits if the user hits cancel,
then Pan will never default to $PWD, $HOME, or anywhere else...