GNOME Bugzilla – Bug 605402
seek Subtitle->Time; Time->Subtitle with kb shortcut; Follow subs.
Last modified: 2010-06-11 11:54:17 UTC
i.e. Sub->Time : ctrl-T will seek movie time to selected subtitle time. Time->Sub : ctrl-shft-T will select subtitle corresponding to movie time. Follow subs (toggle on menu or checkbox) : will scroll and select sub while movie plays. ------------ Objective: Minimize mouse usage while enhancing keyboard only speedy subtitling.
Created attachment 162598 [details] [review] Patch to this bug The Shortcuts were modified to ctrl-p and ctrl-shift-p.
Created attachment 162600 [details] [review] THe last patch was incomplete THe last patch was incomplete because I forgot to add the others files.
Review of attachment 162598 [details] [review]: Inline comments added. ::: src/GnomeSubtitles/Ui/VideoPreview/SubtitleTracker.cs @@ +15,3 @@ +using System; +using SubLib.Core.Domain; +using GnomeSubtitles.Core; Variables should start with lower case letter @@ +31,3 @@ + } + + public int FindSubtitleNearPosition(TimeSpan position){ The "position" timespan isn't being used. Besides that, the use of this method or previousSubtitleIndex is unclear. If we jump from subtitle 1 to 10, previous will be 1, but the "subtitleNearPosition" should always be: 1) if the video position is within a subtitles' boundaries, that subtitle 2) otherwise, the nearest subtitle after the current video position @@ +32,3 @@ + + public int FindSubtitleNearPosition(TimeSpan position){ + if( CurrentSubtitleIndex != -1 ) return CurrentSubtitleIndex; if-then-else should have statements on separate lines (4 lines in this case). Also spaces should be outside parenthesis: if (condition) statement1; else statement2; @@ +49,3 @@ + + + private void setCurrentSubtitle (int index) { Methods should start with capital letter
Review of attachment 162600 [details] [review]: ::: src/Glade/MainWindow.glade @@ -805,3 @@ <accelerator key="F5" signal="activate"/> <signal name="activate" handler="OnVideoPlayPause"/> - <accelerator key="p" signal="activate" modifiers="GDK_CONTROL_MASK"/> The Ctrl+P key should be retained for Play. Suggestions: Ctrl+R and Ctrl+J are free. @@ +889,3 @@ + </child> + </widget> + <accelerator key="p" signal="activate" modifiers="GDK_SHIFT_MASK | GDK_CONTROL_MASK"/> "Selection to Position" might not be understandable by those unfamiliar with these terms. Suggestion: "Select Nearest Subtitle". ::: src/GnomeSubtitles/Ui/Menus.cs @@ +284,1 @@ private void SetVideoSelectionDependentSensitivity (bool sensitivity) { This methods sets sensitivity for items that depend on subtitles being selected. Selecting a subtitle from the video should depend on the video + subtitle file being open, so it should be updated in SetVideoSensitivity and SetDocumentSensitivity.
Created attachment 163314 [details] [review] Correct the bugs 453220 and 605442 Include the codes to bugs 453220 and 605442, because was commons changes to this features. It misses put the sensibility codes of menu controls in the correct place.
Review of attachment 163314 [details] [review]: OK
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.