Opened 4 years ago

Closed 4 years ago

#12241 closed enhancement (fixed)

[ShowImage] add Get info shortcut (easy)

Reported by: diver Owned by: leavengood
Priority: normal Milestone: Unscheduled
Component: Applications/ShowImage Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

It would be nice to be able to press Alt+I to get Tracker's info window regarding currently displayed image.

Attachments (2)

0001-12241-Adding-Get-Info-option-to-show-image-tool.patch (2.3 KB) - added by artcodex 4 years ago.
Adding Get Info to show image tool
0001-ShowImage-Adds-an-option-to-launch-tracker-file-info.patch (2.4 KB) - added by artcodex 4 years ago.
Updated to conform to code review guidelines and with fixes

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by artcodex

Adding Get Info to show image tool

comment:1 Changed 4 years ago by artcodex

Has a Patch: set

comment:2 Changed 4 years ago by artcodex

I wanted to take this on as a good started issue to look at as I'm new to Haiku. I didn't realize the exact process yet, but started looking at this, just to get my feet wet in the source code and the system. Hopefully this item is still up for grabs. I added a patch set of the work I've done so far for this.

comment:3 Changed 4 years ago by waddlesplash

Looks like your coding style doesn't conform to our guidelines (https://www.haiku-os.org/development/coding-guidelines). Also, your commit message should contain the name of the app ("ShowImage") first, followed by a colon, and a complete sentence ending in a period. The ticket number should be in the body of the commit details.

comment:4 Changed 4 years ago by pulkomandy

More complete code review:

  • Indent with tabs (4 columns per tab)
  • I would try to fit the menu item in one of the existing parts of the menu (maybe just under "move to trash"?). Putting it in its own section between two separators isn't useful.
  • Two blank lines between functions
  • Why inserting the method declaration between PrepareForPrint and Print? These two sould like they should stay together in the group of "printing" functions.

Changed 4 years ago by artcodex

Updated to conform to code review guidelines and with fixes

comment:5 Changed 4 years ago by artcodex

I updated the patch to conform to coding guidelines and with above suggestions for better placement. Also updated the commit message. Let me know if there is further feedback.

comment:6 Changed 4 years ago by pulkomandy

Resolution: fixed
Status: newclosed

Applied in hrev50087. Thanks!

Note: See TracTickets for help on using tickets.