Clean up TimeEdit

Remove some unused or constant arguments and simplify some overly
convoluted logic.

Check for whether timecodes are open rather than whether video is open
to determine if by-frame mode is enabled.

Operate on a project context rather than using VideoContext::Get().

Use non-event-generating setter methods rather than a boolean ready
variable.

Make all member variables private and add setters rather than relying on
the client code calling Update when appropriate.

Eliminate flickering in overwrite mode.

Originally committed to SVN as r6056.

aegisub/src/dialog_jumpto.cpp

@@ -71,7 +71,7 @@ DialogJumpTo::DialogJumpTo(agi::Context *c)
 	wxStaticText *LabelTime = new wxStaticText(this,-1,_("Time: "),wxDefaultPosition,wxSize(60,20));
 	JumpFrame = new wxTextCtrl(this,-1,wxString::Format("%ld",jumpframe),wxDefaultPosition,wxSize(60,20),wxTE_PROCESS_ENTER, NumValidator());
 	JumpFrame->SetMaxLength(maxLength.size());
-	JumpTime = new TimeEdit(this,-1,jumptime.GetASSFormated(),wxDefaultPosition,wxSize(60,20),wxTE_PROCESS_ENTER);
+	JumpTime = new TimeEdit(this, -1, c, jumptime.GetASSFormated(), wxSize(60,20));
 	wxSizer *FrameSizer = new wxBoxSizer(wxHORIZONTAL);
 	wxSizer *TimeSizer = new wxBoxSizer(wxHORIZONTAL);
 	FrameSizer->Add(LabelFrame,0,wxALIGN_CENTER_VERTICAL,0);
@@ -112,7 +112,7 @@ void DialogJumpTo::OnOK(wxCommandEvent &) {
 }
 
 void DialogJumpTo::OnEditTime (wxCommandEvent &) {
-	long newframe = c->videoController->FrameAtTime(JumpTime->time.GetMS());
+	long newframe = c->videoController->FrameAtTime(JumpTime->GetMS());
 	if (jumpframe != newframe) {
 		jumpframe = newframe;
 		JumpFrame->ChangeValue(wxString::Format("%i", jumpframe));
@@ -121,9 +121,5 @@ void DialogJumpTo::OnEditTime (wxCommandEvent &) {
 
 void DialogJumpTo::OnEditFrame (wxCommandEvent &event) {
 	JumpFrame->GetValue().ToLong(&jumpframe);
-	int newtime = c->videoController->TimeAtFrame(jumpframe);
+	JumpTime->SetMS(c->videoController->TimeAtFrame(jumpframe));
-	if (JumpTime->time.GetMS() != newtime) {
-		JumpTime->time.SetMS(newtime);
-		JumpTime->ChangeValue(JumpTime->time.GetASSFormated());
-	}
 }

aegisub/src/dialog_shift_times.cpp

@@ -73,7 +73,7 @@ DialogShiftTimes::DialogShiftTimes(agi::Context *context)
 	shift_by_frames->SetToolTip(_("Shift by frames"));
 	shift_by_frames->Bind(wxEVT_COMMAND_RADIOBUTTON_SELECTED, &DialogShiftTimes::OnByFrames, this);
 
-	shift_time = new TimeEdit(this, -1);
+	shift_time = new TimeEdit(this, -1, context);
 	shift_time->SetToolTip(_("Enter time in h:mm:ss.cs notation"));
 
 	shift_frames = new wxTextCtrl(this, -1);
@@ -193,7 +193,7 @@ void DialogShiftTimes::OnClose(wxCommandEvent &) {
 	long shift;
 	shift_frames->GetValue().ToLong(&shift);
 
-	OPT_SET("Tool/Shift Times/Time")->SetInt(shift_time->time.GetMS());
+	OPT_SET("Tool/Shift Times/Time")->SetInt(shift_time->GetMS());
 	OPT_SET("Tool/Shift Times/Frames")->SetInt(shift);
 	OPT_SET("Tool/Shift Times/ByTime")->SetBool(shift_by_time->GetValue());
 	OPT_SET("Tool/Shift Times/Type")->SetInt(time_fields->GetSelection());
@@ -289,7 +289,7 @@ void DialogShiftTimes::Process(wxCommandEvent &) {
 
 	long shift;
 	if (by_time) {
-		shift = shift_time->time.GetMS();
+		shift = shift_time->GetMS();
 		if (shift == 0) {
 			Close();
 			return;

aegisub/src/subs_edit_box.cpp

@@ -177,10 +177,9 @@ SubsEditBox::SubsEditBox(wxWindow *parent, agi::Context *context)
 
 	// Middle controls
 	Layer = new wxSpinCtrl(this,wxID_ANY,"",wxDefaultPosition,wxSize(50,-1),wxSP_ARROW_KEYS,0,0x7FFFFFFF,0);
-	StartTime = new TimeEdit(this,wxID_ANY,"",wxDefaultPosition,wxSize(75,-1),wxTE_PROCESS_ENTER);
+	StartTime = new TimeEdit(this, wxID_ANY, context, "", wxSize(75,-1));
-	EndTime = new TimeEdit(this,wxID_ANY,"",wxDefaultPosition,wxSize(75,-1),wxTE_PROCESS_ENTER);
+	EndTime = new TimeEdit(this, wxID_ANY, context, "", wxSize(75,-1), true);
-	EndTime->isEnd = true;
+	Duration = new TimeEdit(this,wxID_ANY, context,"",wxSize(75,-1));
-	Duration = new TimeEdit(this,wxID_ANY,"",wxDefaultPosition,wxSize(75,-1),wxTE_PROCESS_ENTER);
 	MarginL = new wxTextCtrl(this,wxID_ANY,"",wxDefaultPosition,wxSize(40,-1),wxTE_CENTRE | wxTE_PROCESS_ENTER,NumValidator());
 	MarginL->SetMaxLength(4);
 	MarginR = new wxTextCtrl(this,wxID_ANY,"",wxDefaultPosition,wxSize(40,-1),wxTE_CENTRE | wxTE_PROCESS_ENTER,NumValidator());
@@ -520,21 +519,21 @@ void SubsEditBox::CommitText(wxString desc) {
 }
 
 void SubsEditBox::CommitTimes(TimeField field) {
-	Duration->SetTime(EndTime->time - StartTime->time);
+	Duration->SetTime(EndTime->GetTime() - StartTime->GetTime());
 
 	// Update lines
 	for (Selection::iterator cur = sel.begin(); cur != sel.end(); ++cur) {
 		switch (field) {
 			case TIME_START:
-				(*cur)->Start = StartTime->time;
+				(*cur)->Start = StartTime->GetTime();
 				if ((*cur)->Start > (*cur)->End) (*cur)->End = (*cur)->Start;
 				break;
 			case TIME_END:
-				(*cur)->End = EndTime->time;
+				(*cur)->End = EndTime->GetTime();
 				if ((*cur)->Start > (*cur)->End) (*cur)->Start = (*cur)->End;
 				break;
 			case TIME_DURATION:
-				(*cur)->End = (*cur)->Start + Duration->time;
+				(*cur)->End = (*cur)->Start + Duration->GetTime();
 				break;
 		}
 	}
@@ -634,17 +633,17 @@ void SubsEditBox::OnLayerEnter(wxCommandEvent &) {
 }
 
 void SubsEditBox::OnStartTimeChange(wxCommandEvent &) {
-	if (StartTime->time > EndTime->time) EndTime->SetTime(StartTime->time);
+	if (StartTime->GetTime() > EndTime->GetTime()) EndTime->SetTime(StartTime->GetTime());
 	CommitTimes(TIME_START);
 }
 
 void SubsEditBox::OnEndTimeChange(wxCommandEvent &) {
-	if (StartTime->time > EndTime->time) StartTime->SetTime(EndTime->time);
+	if (StartTime->GetTime() > EndTime->GetTime()) StartTime->SetTime(EndTime->GetTime());
 	CommitTimes(TIME_END);
 }
 
 void SubsEditBox::OnDurationChange(wxCommandEvent &) {
-	EndTime->SetTime(StartTime->time + Duration->time);
+	EndTime->SetTime(StartTime->GetTime() + Duration->GetTime());
 	CommitTimes(TIME_DURATION);
 }
 void SubsEditBox::OnMarginLChange(wxCommandEvent &) {

aegisub/src/timeedit_ctrl.cpp

@@ -36,7 +36,11 @@
 
 #include "config.h"
 
+#include "timeedit_ctrl.h"
+
 #ifndef AGI_PRE
+#include <tr1/functional>
+
 #include <wx/clipbrd.h>
 #include <wx/dataobj.h>
 #include <wx/menu.h>
@@ -45,31 +49,30 @@
 
 #include "ass_time.h"
 #include "compat.h"
+#include "include/aegisub/context.h"
 #include "main.h"
-#include "timeedit_ctrl.h"
 #include "video_context.h"
 
 #ifdef __WXGTK__
 /// Use the multiline style only on wxGTK to workaround some wxGTK bugs with the default singleline style.
 #define TimeEditWindowStyle wxTE_MULTILINE | wxTE_CENTRE
 #else
-
 /// All other platforms than wxGTK.
 #define TimeEditWindowStyle wxTE_CENTRE
 #endif
 
-/// @brief Constructor 
+enum {
-/// @param parent    
+	Time_Edit_Copy = 1320,
-/// @param id        
+	Time_Edit_Paste
-/// @param value     
+};
-/// @param pos       
+
-/// @param size      
+TimeEdit::TimeEdit(wxWindow* parent, wxWindowID id, agi::Context *c, const wxString& value, const wxSize& size, bool asEnd)
-/// @param style     
+: wxTextCtrl(parent, id, value, wxDefaultPosition, size,TimeEditWindowStyle | wxTE_PROCESS_ENTER)
-/// @param validator 
+, c(c)
-/// @param name      
+, byFrame(false)
-///
+, isEnd(asEnd)
-TimeEdit::TimeEdit(wxWindow* parent, wxWindowID id, const wxString& value, const wxPoint& pos, const wxSize& size, long style, const wxValidator& validator, const wxString& name) :
+, insert(!OPT_GET("Subtitle/Time Edit/Insert Mode")->GetBool())
-wxTextCtrl(parent,id,value,pos,size,TimeEditWindowStyle | style,validator,name)
+, insert_opt(OPT_SUB("Subtitle/Time Edit/Insert Mode", &TimeEdit::OnInsertChanged, this))
 {
 	// Set validator
 	wxTextValidator val(wxFILTER_INCLUDE_CHAR_LIST);
@@ -99,227 +102,146 @@ wxTextCtrl(parent,id,value,pos,size,TimeEditWindowStyle | style,validator,name)
 	h += 8;
 	SetSizeHints(w,h,w,h);
 #endif
-	ready = true;
-	byFrame = false;
-	isEnd = false;
 
 	Bind(wxEVT_COMMAND_TEXT_UPDATED, &TimeEdit::OnModified, this);
 	Bind(wxEVT_CONTEXT_MENU, &TimeEdit::OnContextMenu, this);
+	Bind(wxEVT_KEY_DOWN, &TimeEdit::OnKeyDown, this);
+	Bind(wxEVT_COMMAND_MENU_SELECTED, std::tr1::bind(&TimeEdit::CopyTime, this), Time_Edit_Copy);
+	Bind(wxEVT_COMMAND_MENU_SELECTED, std::tr1::bind(&TimeEdit::PasteTime, this), Time_Edit_Paste);
 }
 
-BEGIN_EVENT_TABLE(TimeEdit, wxTextCtrl)
+void TimeEdit::SetMS(int ms) {
-	EVT_KEY_DOWN(TimeEdit::OnKeyDown)
+	if (ms != time.GetMS()) {
-	EVT_MENU(Time_Edit_Copy,TimeEdit::OnCopy)
+		time.SetMS(ms);
-	EVT_MENU(Time_Edit_Paste,TimeEdit::OnPaste)
-END_EVENT_TABLE()
-
-/// @brief Modified event 
-/// @param event 
-void TimeEdit::OnModified(wxCommandEvent &event) {
-	event.Skip();
-	Modified();
-}
-
-/// @brief Modified function 
-void TimeEdit::Modified() {
-	if (!ready) return;
-	ready = false;
-	
-	if (byFrame) Update();
-	else UpdateTime(true);
-
-	ready = true;
-}
-
-/// @brief Set time and update stuff 
-/// @param ms          
-/// @param setModified 
-void TimeEdit::SetTime(AssTime newTime) {
-	if (newTime != time) {
-		time = newTime;
 		UpdateText();
 	}
 }
 
-/// @brief Toggles between set by frame and time 
-/// @param enable 
 void TimeEdit::SetByFrame(bool enableByFrame) {
 	if (enableByFrame == byFrame) return;
 
-	if (enableByFrame) {
+	byFrame = enableByFrame && c->videoController->TimecodesLoaded();
-		if (VideoContext::Get()->IsLoaded()) {
-			byFrame = true;
 	UpdateText();
 }
-	}
-	else {
-		byFrame = false;
-		UpdateText();
-	}
-}
 
-/// @brief Update text to reflect time value 
-void TimeEdit::UpdateText() {
-	ready = false;
-	if (byFrame) {
-		int frame_n = VideoContext::Get()->FrameAtTime(time.GetMS(),isEnd ? agi::vfr::END : agi::vfr::START);
-		ChangeValue(wxString::Format("%i", frame_n));
-	}
-	else ChangeValue(time.GetASSFormated());
-	ready = true;
-}
 
-/// @brief Update 
+void TimeEdit::OnModified(wxCommandEvent &event) {
-void TimeEdit::Update() {
+	event.Skip();
 	if (byFrame) {
 		long temp;
 		GetValue().ToLong(&temp);
-		SetTime(VideoContext::Get()->TimeAtFrame(temp,isEnd ? agi::vfr::END : agi::vfr::START));
+		SetTime(c->videoController->TimeAtFrame(temp, isEnd ? agi::vfr::END : agi::vfr::START));
+	}
+	else if (insert)
+		time.ParseASS(GetValue());
 }
 
-	// Update time if not on insertion mode
-	else if (!OPT_GET("Subtitle/Time Edit/Insert Mode")->GetBool()) {
-		UpdateTime();
-	}
-}
 
-/// @brief Reads value from a text control and update it 
+void TimeEdit::UpdateText() {
-/// @param byUser 
+	if (byFrame)
-void TimeEdit::UpdateTime(bool byUser) {
+		ChangeValue(wxString::Format("%d", c->videoController->FrameAtTime(time.GetMS(), isEnd ? agi::vfr::END : agi::vfr::START)));
-	bool insertion = OPT_GET("Subtitle/Time Edit/Insert Mode")->GetBool();
+	else
-	wxString text = GetValue();
-	long start=0,end=0;
-	if (insertion && byUser) {
-		GetSelection(&start,&end);
-		if (start == end) {
-			wxString nextChar = text.Mid(start,1);
-			if (nextChar == ":" || nextChar == ".") {
-				wxString temp = text;
-				text = temp.Left(start-1);
-				text += nextChar;
-				text += temp.Mid(start-1,1);
-				text += temp.Mid(start+2);
-				start++;
-				end++;
-			}
-			else if (nextChar.IsEmpty()) text.Remove(start-1,1);
-			else text.Remove(start,1);
-		}
-	}
-
-	// Update time
-	time.ParseASS(text);
-	if (insertion) {
 		ChangeValue(time.GetASSFormated());
-		SetSelection(start,end);
-	}
 }
 
-/// @brief Key pressed 
-/// @param event 
 void TimeEdit::OnKeyDown(wxKeyEvent &event) {
-	// Get key ID
 	int key = event.GetKeyCode();
-	bool insertMode = OPT_GET("Subtitle/Time Edit/Insert Mode")->GetBool();
+	if (event.CmdDown()) {
-	Refresh();
+		if (key == 'C' || key == 'X')
-
-	// Check if it's an acceptable key
-	if (!event.CmdDown()) {
-		if (byFrame || !insertMode || (key != WXK_BACK && key != WXK_DELETE)) {
-			// Reset selection first, if necessary
-			if (!byFrame && insertMode) {
-				long from=0,to=0;
-				GetSelection(&from,&to);
-				if (to != from) SetSelection(from,from);
-			}
-
-			// Allow it through
-			event.Skip();
-		}
-	}
-	else {
-		if (key == 'C' || key == 'X') {
 			CopyTime();
-		}
+		else if (key == 'V')
-		else if (key == 'V') {
 			PasteTime();
-		}
+		else
-		else {
 			event.Skip();
 	}
+	else {
+		// Translate numpad presses to normal numbers
+		if (key >= WXK_NUMPAD0 && key <= WXK_NUMPAD9)
+			key += '0' - WXK_NUMPAD0;
+
+		// If overwriting is disabled, we're in frame mode, or it's a key we
+		// don't care about just let the standard processing happen
+		event.Skip();
+		if (byFrame) return;
+		if (insert) return;
+		if ((key < '0' || key > '9') && key != WXK_BACK && key != WXK_DELETE && key != ';' && key != '.') return;
+
+		event.Skip(false);
+
+		long start = GetInsertionPoint();
+		wxString text = GetValue();
+
+		// Delete does nothing
+		if (key == WXK_DELETE) return;
+		// Back just moves cursor back one without deleting
+		if (key == WXK_BACK) {
+			if (start > 0)
+				SetInsertionPoint(start - 1);
+			return;
+		}
+		// Cursor is at the end so do nothing
+		if (start >= (long)text.size()) return;
+
+		// If the cursor is at punctuation, move it forward to the next digit
+		if (text[start] == ':' || text[start] == '.')
+			++start;
+
+		// : and . hop over punctuation but never insert anything
+		if (key == ';' || key == '.') {
+			SetInsertionPoint(start);
+			return;
+		}
+
+		// Overwrite the digit
+		time.ParseASS(text.Left(start) + (char)key + text.Mid(start + 1));
+		SetValue(time.GetASSFormated());
+		SetInsertionPoint(start + 1);
 	}
 }
 
-///// Mouse/copy/paste events down here /////
+void TimeEdit::OnInsertChanged(agi::OptionValue const& opt) {
+	insert = !opt.GetBool();
+}
+
+void TimeEdit::OnContextMenu(wxContextMenuEvent &evt) {
+	if (byFrame || insert) {
+		evt.Skip();
+		return;
+	}
 
-/// @brief Mouse event 
-/// @param event 
-void TimeEdit::OnContextMenu(wxContextMenuEvent &) {
-	if (!byFrame && OPT_GET("Subtitle/Time Edit/Insert Mode")->GetBool()) {
 	wxMenu menu;
 	menu.Append(Time_Edit_Copy, _("&Copy"));
 	menu.Append(Time_Edit_Paste, _("&Paste"));
 	PopupMenu(&menu);
 }
-}
 
-/// @brief Menu Copy 
-void TimeEdit::OnCopy(wxCommandEvent &) {
-	SetFocus();
-	SetSelection(0,GetValue().Length());
-	CopyTime();
-	Refresh();
-}
-
-/// @brief Menu Paste 
-void TimeEdit::OnPaste(wxCommandEvent &) {
-	SetFocus();
-	PasteTime();
-	Refresh();
-}
-
-/// @brief Copy to clipboard 
 void TimeEdit::CopyTime() {
-	if (byFrame) {
-		Copy();
-		return;
-	}
-
-	// Time
 	if (wxTheClipboard->Open()) {
-		wxTheClipboard->SetData(new wxTextDataObject(GetStringSelection()));
+		wxTheClipboard->SetData(new wxTextDataObject(GetValue()));
 		wxTheClipboard->Close();
 	}
 }
 
-/// @brief Paste from clipboard 
 void TimeEdit::PasteTime() {
 	if (byFrame) {
 		Paste();
 		return;
 	}
 
-	// Time
 	if (wxTheClipboard->Open()) {
-		// Read text
 		wxString text;
 		if (wxTheClipboard->IsSupported(wxDF_TEXT)) {
 			wxTextDataObject data;
 			wxTheClipboard->GetData(data);
-			text = data.GetText();
+			text = data.GetText().Trim(false).Trim(true);
-			text.Trim(false).Trim(true);
 		}
 		wxTheClipboard->Close();
 
-		// Paste time
 		AssTime tempTime;
 		tempTime.ParseASS(text);
 		if (tempTime.GetASSFormated() == text) {
-			ready = false;
+			SetTime(tempTime);
-			SetValue(text);
+			SetSelection(0, GetValue().size());
-			SetSelection(0,GetValue().Length());
-			ready = true;
-			Modified();
 		}
 	}
 }

aegisub/src/timeedit_ctrl.h

@@ -34,65 +34,64 @@
 /// @ingroup custom_control
 ///
 
-
-#pragma once
-
-
-////////////
-// Includes
 #ifndef AGI_PRE
 #include <wx/textctrl.h>
 #endif
 
 #include "ass_time.h"
 
+#include <libaegisub/signal.h>
 
-/// @class TimeEdit
+namespace agi {
-/// @brief DOCME
+	class OptionValue;
+	struct Context;
+}
+
+/// @brief A text edit control for editing AssTime objects
 ///
+/// This control constrains values to valid times, and can display the time
+/// being edited as either a h:mm:ss.cc formatted time, or a frame number
 class TimeEdit : public wxTextCtrl {
-private:
+	agi::Context *c; ///< Project context
-	/// DOCME
+	bool byFrame;    ///< Is the time displayed as a frame number?
-	bool byFrame;
+	bool isEnd;      ///< Should the time be treated as an end time for time <-> frame conversions?
+	AssTime time;    ///< The time, which may be displayed as either a frame number or time
+	bool insert;     ///< If true, disable overwriting behavior in time mode
 
-	/// DOCME
+	agi::signal::Connection insert_opt;
-	bool ready;
 
-	void UpdateText();
 	void CopyTime();
 	void PasteTime();
 
-	/// DOCME
+	/// Set the value of the text box from the current time and byFrame setting
-	void UpdateTime(bool byUser=true);
+	void UpdateText();
 
-	/// DOCME
-	void OnModified(wxCommandEvent &event);
 	void OnContextMenu(wxContextMenuEvent &event);
+	void OnInsertChanged(agi::OptionValue const& opt);
 	void OnKeyDown(wxKeyEvent &event);
-	void OnCopy(wxCommandEvent &event);
+	void OnModified(wxCommandEvent &event);
-	void OnPaste(wxCommandEvent &event);
 
-	void Modified();
 public:
+	/// Get the current time as an AssTime object
+	AssTime GetTime() const { return time; }
+	/// Set the time
+	void SetTime(AssTime time) { SetMS(time.GetMS()); }
 
-	/// DOCME
+	/// Get the current time as milliseconds
-	AssTime time;
+	int GetMS() const { return time.GetMS(); }
+	/// Set the time to the specified milliseconds
+	void SetMS(int ms);
 
-	/// DOCME
+	/// Set whether the time is displayed as a time or the corresponding frame number
-	bool isEnd;
+	/// @param enableByFrame If true, frame numbers are displayed
+	void SetByFrame(bool enableByFrame);
 
-
+	/// Constructor
-	/// DOCME
+	/// @param parent Parent window
-	TimeEdit(wxWindow* parent, wxWindowID id, const wxString& value = "", const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxDefaultSize, long style = 0, const wxValidator& validator = wxDefaultValidator, const wxString& name = wxTextCtrlNameStr);
+	/// @param id Window id
-
+	/// @param c Project context
-	void SetByFrame(bool enable);
+	/// @param value Initial value. Must be a valid time string or empty
-	void SetTime(AssTime time);
+	/// @param size Initial control size
-	void Update();
+	/// @param asEnd Treat the time as a line end time (rather than start) for time <-> frame number conversions
-
+	TimeEdit(wxWindow* parent, wxWindowID id, agi::Context *c, const wxString& value = "", const wxSize& size = wxDefaultSize, bool asEnd = false);
-	DECLARE_EVENT_TABLE()
-};
-
-enum {
-	Time_Edit_Copy = 1320,
-	Time_Edit_Paste
 };