Hi Denis, Daniel,
I needed this feature badly .. so got my hands dirty .. and now I actually
have something thats working.
This is my first Open Source contribution .. infact first mail to any list
.. hope the community takes me lively .. and I must accept the
implementation is not that quiet polished.

Firstly, instead of changing the MRU file structure, I went ahead and
created another MRU pagefile (or in the final implementation can be
individual file config.. ie serialize data like page no, exact view
coordinates etc). The rational behind creating a new file is that, this way
the program will be backward and forward compatible ... else for the first
time use of new version, users will loose MRU data .. or the related code
structure will get complicated.

Secondly, I saw an issue while developing. I was stuck at an issue. For
some files the pages were loading properly .. while for some it wasn't.
After throwing in some custom logs I go these :

1.   ---->open_journal
2.   new_mru_entry
3.   /home/indranil/Name_changed.pdf.xoj
4.   ## 110
5.   update_mru_menu
6.   !!! _do_switch_page (on_vscroll_changed) (221)
7.   !!! _do_switch_page (on_vscroll_changed) (0)
8.  !!! _do_switch_page (open_journal) (0)

At this stage inside  open_journal() at the end I am calling
do_switch_page(ui.mru[0].page, TRUE, TRUE);

at (4) .. I got the proper last saved page number .. but before my
do_switch_page() at (8) is called we are getting  scroll signals and in the
scroll callback its going first to the last page and then to the first page
(without any scroll)  (happening 20% of the time) ... now as
 ui.mru[0].page   is also updated inside do_switch_page()  to capture any
other page change .. so we are losing the page number.

I have temporarily fixed the page loading in my patch by using a global
variable just in this case... but I think there is some issue with the
scroll callback .. or we are getting spurious scroll signals.  Note: I used
the github's master code for this test on latest ubuntu... maybe this will
not be noticed in the sourceforge's code.


Please check and try the attached patch(on latest sourceforge's code) and
lemme know.  Denis it really bugs if each time we need to go to the page
number .. so it will be a good feature. Further we can put a config to
enable and disable the feature.

Regards,
Indranil

On Mon, Sep 29, 2014 at 3:09 PM, Indranil Sur <indrafor...@gmail.com> wrote:

> Hi Daniel,
>
> I am not sure that inside the xoj file is a particularly logical place
> to save a current page number.
> For one, it causes files with identical contents to become different and
> change for no reason (not ideal e.g. for people who sync files between
> locations or to the cloud). It is also not particularly logical, if you
> e-mail me a xoj file in which you last looked at page 5, that it should
> open on page 5 for me as well.
>
> A more logical location might be in the mru file
> (.xournal/recent-files). Since that one is local to an installation of
> xournal (more or less), it is fairly harmless to extend its format
> (plus, the current xournal doesn't read past 8 lines of the file).
> See init_mru(), new_mru_entry(), save_mru_list() in xo-file.c  (I can't
> understand for the life of me why the reading of the MRU file is done
> with g_io_... instead of plain g_fopen() etc., perhaps I was feeling in
> a mood to explore glib features).
>
> It would be logical to simply store in the MRU list, along with the file
> name, the page number on which it was last opened (in memory, extend the
> MRU data structure to have not just a menu item widget and a file name
> but also a page number; in the MRU file, up to you how to do it, perhaps
> append :1 (or :pagenumber) at the end of each file name in the list
> stored on disk?) -- and if we want to keep page numbers for more than
> the last 8 files, increase the length of the list in memory and in the
> MRU file but not in the menus. Then add to the file-open function a
> feature to look up the file name in the MRU list and see if we have a
> preferred page number for it.
>
> Does this make sense?
>
> Best,
> Denis
>
>
> On 09/07/2014 01:07 AM, D M German wrote:
> >
> > hi Denis,
> >
> > what do you think about saving in the xoj file, the current page?
> >
> > If you agree with it, how would like it implemented? I can do the work.
> >
> > --daniel
> >
> > --
> > Daniel M. German                  "Never underestimate the bandwidth
> >                                     of a station wagon full of
> >     Andrew S. Tanenbaum ->          tapes hurtling down the highway."
> > http://turingmachine.org/
> > http://silvernegative.com/
> > dmg (at) uvic (dot) ca
> > replace (at) with @ and (dot) with .
> >
> >
> >
> > ------------------------------------------------------------------------------
> > Slashdot TV.
> > Video for Nerds.  Stuff that matters.
> > http://tv.slashdot.org/
> > _______________________________________________
> > Xournal-devel mailing list
> > Xournal-devel@...
> > https://lists.sourceforge.net/lists/listinfo/xournal-devel
> >
>
> --
> Denis Auroux
> UC Berkeley, Department of Mathematics     auroux@...
>
> Institut Henri Poincare, Paris             auroux@...
>
>
diff --git a/src/main.c b/src/main.c
index 4348e87..9ca1a5e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -59,6 +59,7 @@ void init_stuff (int argc, char *argv[])
   tmppath = g_build_filename(g_get_home_dir(), CONFIG_DIR, NULL);
   g_mkdir(tmppath, 0700); // safer (MRU data may be confidential)
   ui.mrufile = g_build_filename(tmppath, MRU_FILE, NULL);
+  ui.mrupages = g_build_filename(tmppath, MRU_FILE_PAGES, NULL);
   ui.configfile = g_build_filename(tmppath, CONFIG_FILE, NULL);
   g_free(tmppath);
 
diff --git a/src/xo-callbacks.c b/src/xo-callbacks.c
index 5a5b841..338c00f 100644
--- a/src/xo-callbacks.c
+++ b/src/xo-callbacks.c
@@ -3284,16 +3284,16 @@ on_mru_activate                        (GtkMenuItem     *menuitem,
   for (which = 0 ; which < MRU_SIZE; which++) {
     if (ui.mrumenu[which] == GTK_WIDGET(menuitem)) break;
   }
-  if (which == MRU_SIZE || ui.mru[which] == NULL) return; // not found...
+  if (which == MRU_SIZE || ui.mru[which].valid == 0) return; // not found...
 
   set_cursor_busy(TRUE);
-  success = open_journal(ui.mru[which]);
+  success = open_journal(ui.mru[which].name);
   set_cursor_busy(FALSE);
   if (success) return;
 
   /* open failed */
   dialog = gtk_message_dialog_new(GTK_WINDOW (winMain), GTK_DIALOG_DESTROY_WITH_PARENT,
-    GTK_MESSAGE_ERROR, GTK_BUTTONS_OK, _("Error opening file '%s'"), ui.mru[which]);
+    GTK_MESSAGE_ERROR, GTK_BUTTONS_OK, _("Error opening file '%s'"), ui.mru[which].name);
   gtk_dialog_run(GTK_DIALOG(dialog));
   gtk_widget_destroy(dialog);
   delete_mru_entry(which);
diff --git a/src/xo-file.c b/src/xo-file.c
index 4b3712a..996dd7d 100644
--- a/src/xo-file.c
+++ b/src/xo-file.c
@@ -71,6 +71,8 @@ gzFile gzopen_wrapper(const char *path, const char *mode)
 #endif
 }
 
+int load_pg;
+
 // creates a new empty journal
 
 void new_journal(void)
@@ -1176,6 +1178,9 @@ gboolean open_journal(char *filename)
 
   g_free(filename_actual);
   ui.need_autosave = !ui.saved;
+
+  do_switch_page(load_pg, TRUE, TRUE);
+
   return TRUE;
 }
 
@@ -1595,25 +1600,46 @@ void init_mru(void)
   int i;
   gsize lfptr;
   char s[5];
-  GIOChannel *f;
+  GIOChannel *f, *f_p;
   gchar *str;
   GIOStatus status;
+  int skip_page_read;
   
   g_strlcpy(s, "mru0", 5);
   for (s[3]='0', i=0; i<MRU_SIZE; s[3]++, i++) {
     ui.mrumenu[i] = GET_COMPONENT(s);
-    ui.mru[i] = NULL;
+    //ui.mru[i].name = NULL;
   }
+  memset(ui.mru, 0, sizeof(ui.mru));
+  
   f = g_io_channel_new_file(ui.mrufile, "r", NULL);
   if (f) status = G_IO_STATUS_NORMAL;
   else status = G_IO_STATUS_ERROR;
+  
+  f_p = g_io_channel_new_file(ui.mrupages, "r", NULL);
+  if (f_p) skip_page_read = 0;
+  else skip_page_read = 1;
+  
   i = 0;
   while (status == G_IO_STATUS_NORMAL && i<MRU_SIZE) {
     lfptr = 0;
     status = g_io_channel_read_line(f, &str, NULL, &lfptr, NULL);
     if (status == G_IO_STATUS_NORMAL && lfptr>0) {
       str[lfptr] = 0;
-      ui.mru[i] = str;
+      ui.mru[i].name = str;
+      ui.mru[i].valid = 1;
+      if(!skip_page_read) {
+         status = g_io_channel_read_line(f_p, &str, NULL, &lfptr, NULL);
+         if (status == G_IO_STATUS_NORMAL && lfptr>0) {
+//          here parsing logic could be added if multiple parameters are saved
+	    str[lfptr] = 0;
+	    ui.mru[i].page = atoi(str);
+         } else {
+	    ui.mru[i].page = 0;
+         }
+      } else {
+	 ui.mru[i].page = 0;
+      }
       i++;
     }
   }
@@ -1621,6 +1647,7 @@ void init_mru(void)
     g_io_channel_shutdown(f, FALSE, NULL);
     g_io_channel_unref(f);
   }
+  load_pg=ui.mru[0].page;
   update_mru_menu();
 }
 
@@ -1631,9 +1658,9 @@ void update_mru_menu(void)
   gchar *tmp;
   
   for (i=0; i<MRU_SIZE; i++) {
-    if (ui.mru[i]!=NULL) {
+    if (ui.mru[i].valid!=0) {
       tmp = g_strdup_printf("_%d %s", i+1,
-               g_strjoinv("__", g_strsplit_set(xo_basename(ui.mru[i], FALSE),"_",-1)));
+               g_strjoinv("__", g_strsplit_set(xo_basename(ui.mru[i].name, FALSE),"_",-1)));
       gtk_label_set_text_with_mnemonic(GTK_LABEL(gtk_bin_get_child(GTK_BIN(ui.mrumenu[i]))),
           tmp);
       g_free(tmp);
@@ -1645,19 +1672,30 @@ void update_mru_menu(void)
   gtk_widget_set_sensitive(GET_COMPONENT("fileRecentFiles"), anyone);
 }
 
+void swap_mru_entry(struct Mru *to, struct Mru *frm) 
+{
+  to->valid=frm->valid;
+  to->name=frm->name;
+  to->page=frm->page;
+}
+
 void new_mru_entry(char *name)
 {
-  int i, j;
+  int i, j, old_pg=0;
   
   for (i=0;i<MRU_SIZE;i++) 
-    if (ui.mru[i]!=NULL && !strcmp(ui.mru[i], name)) {
-      g_free(ui.mru[i]);
-      for (j=i+1; j<MRU_SIZE; j++) ui.mru[j-1] = ui.mru[j];
-      ui.mru[MRU_SIZE-1]=NULL;
+    if (ui.mru[i].valid!=0 && !strcmp(ui.mru[i].name, name)) {
+      g_free(ui.mru[i].name);
+      old_pg=ui.mru[i].page;
+      for (j=i+1; j<MRU_SIZE; j++) swap_mru_entry(&ui.mru[j-1],&ui.mru[j]);
+      ui.mru[MRU_SIZE-1].name=NULL;
     }
-  if (ui.mru[MRU_SIZE-1]!=NULL) g_free(ui.mru[MRU_SIZE-1]);
-  for (j=MRU_SIZE-1; j>=1; j--) ui.mru[j] = ui.mru[j-1];
-  ui.mru[0] = g_strdup(name);
+  if (ui.mru[MRU_SIZE-1].name!=NULL) g_free(ui.mru[MRU_SIZE-1].name);
+  for (j=MRU_SIZE-1; j>=1; j--) swap_mru_entry(&ui.mru[j],&ui.mru[j-1]);
+  ui.mru[0].name = g_strdup(name);
+  ui.mru[0].valid=1;
+  ui.mru[0].page=old_pg;
+  load_pg=ui.mru[0].page;
   update_mru_menu();
 }
 
@@ -1665,23 +1703,32 @@ void delete_mru_entry(int which)
 {
   int i;
   
-  if (ui.mru[which]!=NULL) g_free(ui.mru[which]);
-  for (i=which+1;i<MRU_SIZE;i++) 
-    ui.mru[i-1] = ui.mru[i];
-  ui.mru[MRU_SIZE-1] = NULL;
+  if (ui.mru[which].valid!=0) g_free(ui.mru[which].name);
+  for (i=which+1;i<MRU_SIZE;i++) swap_mru_entry(&ui.mru[i-1],&ui.mru[i]);
+  ui.mru[MRU_SIZE-1].name = NULL;
   update_mru_menu();
 }
 
 void save_mru_list(void)
 {
-  FILE *f;
+  FILE *f, *f_p;
   int i;
   
+  ui.mru[0].page=ui.pageno;
+  
   f = g_fopen(ui.mrufile, "w");
   if (f==NULL) return;
-  for (i=0; i<MRU_SIZE; i++)
-    if (ui.mru[i]!=NULL) fprintf(f, "%s\n", ui.mru[i]);
+  f_p = g_fopen(ui.mrupages, "w");
+  if (f_p==NULL) return;
+  
+  for (i=0; i<MRU_SIZE; i++) {
+    if (ui.mru[i].valid!=0) {
+      fprintf(f, "%s\n", ui.mru[i].name);
+      fprintf(f_p, "%d\n", ui.mru[i].page);
+    }
+  }
   fclose(f);
+  fclose(f_p);
 }
 
 void init_config_default(void)
diff --git a/src/xo-file.h b/src/xo-file.h
index 03556a4..c107b27 100644
--- a/src/xo-file.h
+++ b/src/xo-file.h
@@ -46,6 +46,7 @@ void bgpdf_update_bg(int pageno, struct BgPdfPage *bgpg);
 
 void init_mru(void);
 void update_mru_menu(void);
+void swap_mru_entry(struct Mru *to, struct Mru *frm);
 void new_mru_entry(char *name);
 void delete_mru_entry(int which);
 void save_mru_list(void);
diff --git a/src/xo-misc.c b/src/xo-misc.c
index 195072b..2a72c16 100644
--- a/src/xo-misc.c
+++ b/src/xo-misc.c
@@ -1428,6 +1428,9 @@ void do_switch_page(int pg, gboolean rescroll, gboolean refresh_all)
     else if (!ui.view_continuous)
       gnome_canvas_item_move(GNOME_CANVAS_ITEM(ui.cur_page->group), 0., 0.);
   }
+  
+  // update the current page change to mru data
+  ui.mru[0].page=pg;
 }
 
 void update_page_stuff(void)
diff --git a/src/xournal.h b/src/xournal.h
index 9b74868..d45cedf 100644
--- a/src/xournal.h
+++ b/src/xournal.h
@@ -36,6 +36,7 @@
 
 #define CONFIG_DIR ".xournal"
 #define MRU_FILE "recent-files"
+#define MRU_FILE_PAGES "recent-files-pages"
 #define MRU_SIZE 8 
 #define CONFIG_FILE "config"
 
@@ -247,6 +248,12 @@ typedef struct Selection {
   float move_pagedelta;
 } Selection;
 
+typedef struct Mru {
+  gboolean valid;
+  char *name;
+  int page;
+}Mru;
+
 typedef struct UIData {
   int pageno, layerno; // the current page and layer
   struct Page *cur_page;
@@ -301,8 +308,8 @@ typedef struct UIData {
   GdkPixbuf *pen_cursor_pix, *hiliter_cursor_pix;
   gboolean pen_cursor; // use pencil cursor (default is a dot in current color)
   gboolean progressive_bg; // update PDF bg's one at a time
-  char *mrufile, *configfile; // file names for MRU & config
-  char *mru[MRU_SIZE]; // MRU data
+  char *mrufile, *mrupages, *configfile; // file names for MRU & config
+  struct Mru mru[MRU_SIZE]; // MRU data
   GtkWidget *mrumenu[MRU_SIZE];
   gboolean bg_apply_all_pages;
   int window_default_width, window_default_height, scrollbar_step_increment;
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
Xournal-devel mailing list
Xournal-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xournal-devel

Reply via email to