After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 594244 - Pdf Mod needs a Fullscreen view mode
Pdf Mod needs a Fullscreen view mode
Status: RESOLVED FIXED
Product: pdfmod
Classification: Other
Component: general
git master
Other All
: Normal enhancement
: ---
Assigned To: pdfmod-maint
pdfmod-maint
Depends on:
Blocks:
 
 
Reported: 2009-09-05 17:55 UTC by Andreu Correa Casablanca
Modified: 2009-09-06 00:05 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
This patch adds the fullscreen view mode (3.10 KB, patch)
2009-09-05 21:41 UTC, Andreu Correa Casablanca 		needs-work Details | Review
This patch adds the fullscreen view mode (3.31 KB, patch)
2009-09-05 22:41 UTC, Andreu Correa Casablanca 		needs-work Details | Review
This patch adds the fullscreen view mode (2.87 KB, patch)
2009-09-05 23:09 UTC, Andreu Correa Casablanca 		committed Details | Review

Description Andreu Correa Casablanca 2009-09-05 17:55:53 UTC 
I think it would be interesting that Pdf Mod allow the users to use a fullscreen view mode.
Comment 1 Andreu Correa Casablanca 2009-09-05 21:41:15 UTC 
Created attachment 142565 [details] [review]
This patch adds the fullscreen view mode

This patch adds the fullscreen view mode.
Comment 2 Gabriel Burt 2009-09-05 21:56:18 UTC 
Comment on attachment 142565 [details] [review]
This patch adds the fullscreen view mode

Hey Andreu,

Thanks for the patch.  Please use 4 spaces for indentation everywhere, and fix that one line where all you do is add trailing whitespace.  :)

Also, replace "Fullscreen view" with just "Fullscreen", and add braces around your if/else.
Comment 3 Andreu Correa Casablanca 2009-09-05 22:41:25 UTC 
Created attachment 142566 [details] [review]
This patch adds the fullscreen view mode

This patch adds the fullscreen view mode. ( Correction of the previous patch )
Comment 4 Gabriel Burt 2009-09-05 22:54:17 UTC 
Comment on attachment 142566 [details] [review]
This patch adds the fullscreen view mode

Close, still not following HACKING though.

1. the first change in the patch shouldn't be there
2. put a space between a method name and its arg list like (this)
3. should be } else { on one line
4. should be if ( not if(
Comment 5 Andreu Correa Casablanca 2009-09-05 23:09:22 UTC 
Created attachment 142567 [details] [review]
This patch adds the fullscreen view mode

Another revision of the patch.
Comment 6 Gabriel Burt 2009-09-06 00:05:17 UTC 
Comment on attachment 142567 [details] [review]
This patch adds the fullscreen view mode

Committed, thanks