El Dimecres, 16 d'octubre de 2013, a les 23:19:51, Albert Astals Cid va escriure: > El Dimecres, 16 d'octubre de 2013, a les 18:36:52, Jonathan Doman va escriure: > > > On Oct. 1, 2013, 6:57 p.m., Fabio D'Urso wrote: > > > > Hi! :) > > > > I have neither tested nor read the whole patch in depth, I've only had > > > > a > > > > look at your description and made a few tests. > > > > > > > > We had a discussion at the okular BOF this summer, and we decided that > > > > it's better to avoid having multiple open copies of the same document, > > > > because this creates consistency issues if you are annotating (while > > > > saving, the two instances will overwrite each other's changes). In the > > > > light of this, I'm not sure "Duplicate tab" makes sense the way it is > > > > now. In my opinion, a better approach would be to tie multiple > > > > PageViews to the same Document. However, as the Document class > > > > currently assumes that there is only one PageView, this requires > > > > significant changes, so we had better leave this feature for a > > > > different patch. > > > > > > > > I see a lighter issue with the way "Detach tab" works: if you create a > > > > new process you will 1) loose all cached pixmaps, 2) cause the > > > > Part::queryClose() message to show up if there are unsaved > > > > annotations. > > > > (If you want to reproduce this issue annotate a PDF, click "save as", > > > > open the file you saved, detach its tab). > > > > > > > > Both "Duplicate tab" and "Detach tab" fail at handling input from - > > > > (standard input)> > > > > > > > > > cat somePdfFile.pdf | okular - > > > > > > > > because, by definition, you can't read it twice. > > > > > > > > About the issue with moving menu items, I see that all you need to do > > > > is > > > > to add the confirm_tabs_close checkbox, right? Maybe you can just add > > > > it to the configuration dialog instead or drop it at all (for example > > > > the warnings from document.cpp warnLimitedAnnotSupport() cannot be > > > > reactivated). > > > > So should I remove the duplicate tab feature? It's possible in the master > > code to open the same document in two windows, and it would still be > > possible in this code to manually open the same document to get it in > > another tab. Is that going to be blocked in the future? If not, I don't > > see > > how duplication causes any additional problems. > > > > Do you agree with the logic to open new documents in the MRU existing > > window? No one has given their opinion on this yet. If not, then it's easy > > to keep documents in the same process and I'll do that. > > > > I'll take a look at the other stuff and upload a new diff when I get some > > time. > > Can you please put this comment in reviewboard instead of directly to the > list? It makes it easier to have stuff centralized in one place.
Silly me, this was on the reviewboard, just could not find it ^_^ Sorry, Albert > > Thanks, > Albert > > > - Jonathan > > > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://git.reviewboard.kde.org/r/110914/#review41071 > > ----------------------------------------------------------- > > > > On Aug. 23, 2013, 8:06 p.m., Jonathan Doman wrote: > > > ----------------------------------------------------------- > > > This is an automatically generated e-mail. To reply, visit: > > > http://git.reviewboard.kde.org/r/110914/ > > > ----------------------------------------------------------- > > > > > > (Updated Aug. 23, 2013, 8:06 p.m.) > > > > > > > > > Review request for Okular. > > > > > > > > > Bugs: 155515 > > > > > > http://bugs.kde.org/show_bug.cgi?id=155515 > > > > > > Repository: okular > > > > > > > > > Description > > > ------- > > > > > > This patch adds support for a tabbed interface (multiple documents in > > > one > > window). The core work just adds a tab bar that switches between multiple > > embedded okularparts, but there are many other considerations: > > > - Tab context menu allows for duplicating or detaching (detached tabs > > > start in new okular process) - `okular file.pdf` will open file in > > > existing window if possible, unless --new flag is used. It also selects > > > the most recently raised/activated window to use. This mirrors behavior > > > I expect from browsers and other tabbed interfaces. - Warns when > > > closing > > > window with multiple tabs > > > - No warning is given when opening an already open file. This is the > > > behavior I strongly prefer (and observe in other programs), but will > > > change if there is consensus otherwise.> > > > > > > When selecting different tools in one part, the tool selection > > > propagates > > > to all parts, but the GUI does not reflect that. This bug is present in > > > other programs (e.g. multiple okularparts in Konqueror), so I made no > > > attempt to diagnose or fix. > > > > > > One menu item was added for the multiple tab warning option. When > > > testing > > > this, I noticed that items in the Settings menu seem to move around when > > > switching tabs, and I cannot diagnose or fix this. It seems to be > > > related > > > to XMLGUI bug #64754. > > > > > > My development branch is also hosted at > > > https://github.com/jrmrjnck/okular-tabbed > > > > > > > > > Diffs > > > ----- > > > > > > part.h 4b3aafdb637080ae81eb0e45742f53a34738984d > > > part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 > > > shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 > > > shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 > > > shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 > > > shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 > > > > > > Diff: http://git.reviewboard.kde.org/r/110914/diff/ > > > > > > > > > Testing > > > ------- > > > > > > > > > Thanks, > > > > > > Jonathan Doman > > _______________________________________________ > Okular-devel mailing list > Okular-devel@kde.org > https://mail.kde.org/mailman/listinfo/okular-devel _______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
