Re: [Flightgear-devel] Add screenshot dialog, to select directory
On Mon, Jan 17, 2011 at 4:44 PM, Gijs de Rooy wrote: Hi Melchior, I was about to send an email to the mailing list asking for feedback, so thanks for saving my three minutes :) Shudder! This patch simply duplicates a lot of code that was meant to be *re-used*. Right. So far it was a proof of concept. What it needs now is someone to clean the code up and improve it. I agree that it might have been better to distribute it through a diff, rather than commiting it yet, though; but given the fact that it only adds something (and doesn't break anything else; atleast it should not) it is not a very big deal I think to have it temporarily like this in Git... (don't get me wrong, I won't say it was right to do!) Hi Gijs, Are you actively working on this feature, or expecting someone else to clean it up? In addition to the coding issues that Melchior has highlighted, I've noticed a couple of basic functional problems: 1) Typo in the menu bar Screesnhot (set directory). At minimum you should correct the typo, and I would suggest changing the label to Set Screenshot Directory, though even that wording is a bit kludgy. 2) On my system I can't see any way to go up a directory using the GUI, so I'm stuck inside /tmp/ unless I enter a directory manually. 3) No Close button on the bottom of the dialog (for consistency with the rest of the UI). This should be fixed in both this and the File Selector dialog class. Being very blunt, while a proof of concept is very nice, I think you need to do more this close to the v2.2.0 release. IMO you need to either address these issues yourself, find someone to do so for you, or back it out. You can't just check in such proof of concept without a plan to bring it up to the normal level of quality. Please let me know which way you intend to go, so I can determine whether to update The Manual or not. -Stuart -- Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! Finally, a world-class log management solution at an even better price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer expires February 28th, so secure your free ArcSight Logger TODAY! http://p.sf.net/sfu/arcsight-sfd2d ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Add screenshot dialog, to select directory
Hi Stuart, thanks for taking the time to write this kind message, it's appreciated! Being very blunt, while a proof of concept is very nice, I think you need to do more this close to the v2.2.0 release. You're right. I did not relate it to the release being close for some reason, which does look very stupid now. Commit has been reverted for now, I'll probably come up with something better for a next release; when I have more time to improve it and have it tested before the release. Cheers and once again sorry! Won't happen again. Gijs -- Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! Finally, a world-class log management solution at an even better price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer expires February 28th, so secure your free ArcSight Logger TODAY! http://p.sf.net/sfu/arcsight-sfd2d___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
[Flightgear-devel] Add screenshot dialog, to select directory
* Flightgear-commitlogs -- Saturday 15 January 2011: commit adb57b8154a4a9dad8107c0c6633c276b43c4a20 Author: Gijs de Rooy Date: Sat Jan 15 22:48:43 2011 +0100 Add screenshot dialog, to select directory Shudder! This patch simply duplicates a lot of code that was meant to be *re-used*. A whole copy of the file dialog architecture, only to add one checkbox! A whole Nasal class for screenshot dir selectors, in addition to already existing generic file selectors? Is there no more quality control? Is every newbie now allowed to commit to core parts without any review (except post factum)? m. -- Protect Your Site and Customers from Malware Attacks Learn about various malware tactics and how to avoid them. Understand malware threats, the impact they can have on your business, and how you can protect your company and customers by using code signing. http://p.sf.net/sfu/oracle-sfdevnl ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Add screenshot dialog, to select directory
Hi Melchior, I was about to send an email to the mailing list asking for feedback, so thanks for saving my three minutes :) Shudder! This patch simply duplicates a lot of code that was meant to be *re-used*. Right. So far it was a proof of concept. What it needs now is someone to clean the code up and improve it. I agree that it might have been better to distribute it through a diff, rather than commiting it yet, though; but given the fact that it only adds something (and doesn't break anything else; atleast it should not) it is not a very big deal I think to have it temporarily like this in Git... (don't get me wrong, I won't say it was right to do!) Anyway, next time I'll do it differently for sure; I learn something new every day. Sorry for now. A whole Nasal class for screenshot dir selectors, in addition to already existing generic file selectors? As far as I was able to find out the screen-capture command does not (yet) support filenames. Only directories. So we needed a directory selector, rather than file selector... Nicer solutions are welcome as always. Cheers and once again sorry! Gijs -- Protect Your Site and Customers from Malware Attacks Learn about various malware tactics and how to avoid them. Understand malware threats, the impact they can have on your business, and how you can protect your company and customers by using code signing. http://p.sf.net/sfu/oracle-sfdevnl___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Add screenshot dialog, to select directory
On Jan 17, 2011, at 11:44 AM, Gijs de Rooy wrote: Hi Melchior, I was about to send an email to the mailing list asking for feedback, so thanks for saving my three minutes :) Cheers and once again sorry! Gijs Gijs, You probably know better than I, but do most folks use or need a FlightGear specified screen capture utility? Perhaps I look at it another way, but even if it has one I think I'd still use my computer's screen capture utility. Just an opinion - :) Peter-- Protect Your Site and Customers from Malware Attacks Learn about various malware tactics and how to avoid them. Understand malware threats, the impact they can have on your business, and how you can protect your company and customers by using code signing. http://p.sf.net/sfu/oracle-sfdevnl___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Add screenshot dialog, to select directory
On Mon, Jan 17, 2011 at 6:40 PM, Peter Brown smoothwater...@adelphia.net wrote: You probably know better than I, but do most folks use or need a FlightGear specified screen capture utility? Yes. Perhaps I look at it another way, but even if it has one I think I'd still use my computer's screen capture utility. A built-in screenshot function does have some advantages. For example, it can temporarily hide the menu bar, can be bound to a joystick button, can be triggered from nasal and more such goodies. At least theoretically it could also produce a single image for a multi-display setup. I personally don't need a dialog for it, though. -- Cheers, Csaba/Jester -- Protect Your Site and Customers from Malware Attacks Learn about various malware tactics and how to avoid them. Understand malware threats, the impact they can have on your business, and how you can protect your company and customers by using code signing. http://p.sf.net/sfu/oracle-sfdevnl ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Add screenshot dialog, to select directory
On Jan 17, 2011, at 1:35 PM, Csaba Halász wrote: On Mon, Jan 17, 2011 at 6:40 PM, Peter Brown smoothwater...@adelphia.net wrote: You probably know better than I, but do most folks use or need a FlightGear specified screen capture utility? Yes. Perhaps I look at it another way, but even if it has one I think I'd still use my computer's screen capture utility. A built-in screenshot function does have some advantages. For example, it can temporarily hide the menu bar, can be bound to a joystick button, can be triggered from nasal and more such goodies. At least theoretically it could also produce a single image for a multi-display setup. I personally don't need a dialog for it, though. -- Cheers, Csaba/Jester I see, makes more sense if someone takes constant shots. One the great little tidbits someone already committed was an autohide menu bar. Although a minor enhancement, I find that to be very nice. Thanks, Peter -- Protect Your Site and Customers from Malware Attacks Learn about various malware tactics and how to avoid them. Understand malware threats, the impact they can have on your business, and how you can protect your company and customers by using code signing. http://p.sf.net/sfu/oracle-sfdevnl ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
