Re: [jokosher-devel] [PATCH] Patch for Launchpad bug 85938 - Usability for selection-based editing

["Tom Halligan" <tom halligan gmail com>]

Woops silly me - I have indeed been using Eclipse, with some third-party SVN plugin. I just assumed it was doing things right - obviously not!

Anyway, here's a new patch with your suggestions duly noted and taken care of:
1) (I assume you meant EventLaneViewer.py for this - EventViewer does indeed have a lot of changes) No, nothing changed in EventLaneViewer.py, must have knocked return by mistake or something :P

2) Fixed!

3) I called it DummySplitEvent because it 'emulates' splitting, if you see what I mean. But you're right, it was a bad name, now it's called 'CopySelection'.

4) Done - now works rather well if I do say so myself :P I also included a few try...excepts in __UpdateFadeLevels, as I kept receiving exceptions and broken functionality caused by the iterFadePoints.next() bits when using the new copy functionality. It should work fine now :)

Like I said in the last post - these things only work when you shift-select on an event, then right click. They don't currently work from the Edit menu.

Tom

On Wed, May 21, 2008 at 6:36 PM, Laszlo Pandy <laszlok2 gmail com> wrote:

Hey, thanks Tom. It looks like a pretty good. Here are my comments:

1) Did anything change in EventViewer.py? It looks like just a space was added. Maybe you should revert that file.

2) It looks like you are using Eclipse which has generated a patch with absolute paths in it. This means when I try to apply it it cannot find /home/tom/ and I have to type all the paths manually. In the default SVN client doing an 'svn diff' will give you a patch with paths relative to the current directory like "Jokosher/EventViewer.py". I'm not sure how to do this in Eclipse, but it would be nice if you can.

3) What does the Dummy in DummySplitEvent() mean? Might consider changing the name to make it more clear. Would SplitEventOnSelection() make more sense?

4) In SplitEvent() there is some code for splitting the __fadePointsDict, so that if you have fade points on either side of the split point they will be transferred to the new events. I don't see any of that code in DummySplitEvent(). Has this been overlooked or did I miss something?

Thanks, Laszlo

Tom Halligan wrote:

Posted this to Launchpad RE: Jokosher bug #85938 at https://bugs.launchpad.net/jokosher/+bug/85938 , thought I'd send to the mailing list too:

Small patch going part of the way to resolving this bug - splitting, cutting, copying, and deleting now works on a selection of an event rather than the entire thing from the right-click context menu.

These things still don't work from the edit menu, as Jokosher uses seperate code for the two and I haven't got round to coding the edit menu bits yet.

Also, these things work on pretty much an aesthetic level only - copying does not trim an event down to size properly (Looks ok, but plays the whole event rather than the selected portion), and various other issues related to other Jokosher bugs abound.

It looks nicer though :)

This patch was developed against Jokosher SVN from Revision 1512.

Still finding my way around the Jokosher code - lots of things seem kind of awkward, but that's for another discussion completely!

Peace
Tom

------------------------------------------------------------------------

_______________________________________________
jokosher-devel-list mailing list
jokosher-devel-list gnome org
http://mail.gnome.org/mailman/listinfo/jokosher-devel-list

Index: Jokosher/EventViewer.py
===================================================================
--- Jokosher/EventViewer.py	(revision 1512)
+++ Jokosher/EventViewer.py	(working copy)
@@ -997,7 +997,7 @@
 			widget -- reserved for GTK callbacks, don't use it explicitly.
 			mouse -- GTK mouse event that fired this method call.
 		"""
-		if self.messageID:   #clesr status bar if not already clear
+		if self.messageID:   #clear status bar if not already clear
 			self.mainview.ClearStatusBar(self.messageID)
 			self.messageID = None
 		self.highlightCursor = None
@@ -1013,11 +1013,17 @@
 			gtkevent -- reserved for GTK callbacks, don't use it explicitly.
 			position -- The position in the event to split
 		"""
-		if pos == 0.0:
-			return
-			
-		pos /= float(self.project.viewScale)
-		self.event.SplitEvent(pos)
+		if self.event.selection != [0,0]:
+			self.event.SplitEvent(self.event.selection[1])
+			self.event.SplitEvent(self.event.selection[0])
+			self.event.selection = [0,0]
+			self.HideDrawer()
+		else:
+			if pos == 0.0:
+				return
+			else:
+				pos /= float(self.project.viewScale)
+				self.event.SplitEvent(pos)
 		
 	#_____________________________________________________________________
 	
@@ -1028,8 +1034,16 @@
 		Parameters:
 			gtkevent -- reserved for GTK callbacks, don't use it explicitly.
 		"""
-		self.project.clipboardList = [self.event]
-		self.OnDelete()
+		if self.event.selection != [0,0]:
+			self.event.SplitEvent(self.event.selection[1])
+			e = self.event.SplitEvent(self.event.selection[0])
+			self.project.clipboardList = [e]
+			e.Delete()
+			self.event.selection = [0,0]
+			self.HideDrawer()
+		else:
+			self.project.clipboardList = [self.event]
+			self.OnDelete()
 	
 	#_____________________________________________________________________
 	
@@ -1040,7 +1054,12 @@
 		Parameters:
 			gtkevent -- reserved for GTK callbacks, don't use it explicitly.
 		"""
-		self.project.clipboardList = [self.event]
+		if self.event.selection != [0,0]:
+			e = self.event.CopySelection()
+			self.project.clipboardList = [e]
+			#We shouldn't hide the drawer here, unfriendly behaviour
+		else:
+			self.project.clipboardList = [self.event]
 	
 	#_____________________________________________________________________
 
@@ -1052,7 +1071,14 @@
 		Parameters:
 			event -- reserved for GTK callbacks, don't use it explicitly.
 		"""
-		self.event.Delete()
+		if self.event.selection != [0,0]:
+			self.event.SplitEvent(self.event.selection[1])
+			e = self.event.SplitEvent(self.event.selection[0])
+			e.Delete()
+			self.event.selection = [0,0]
+			self.HideDrawer()
+		else:
+			self.event.Delete()
 	
 	#_____________________________________________________________________
 		
Index: Jokosher/Event.py
===================================================================
--- Jokosher/Event.py	(revision 1512)
+++ Jokosher/Event.py	(working copy)
@@ -312,6 +312,55 @@
 	
 	#_____________________________________________________________________
 	
+	def CopySelection(self, eventID=-1):
+		"""
+		Only for use with a 2-point selection.
+		Essentially performs a 'fake split' and returns a new event
+		which would be the result of splitting an event at the 2 points.
+		
+		This is used when the user shift-drags an event to create a selection,
+		then chooses 'copy' from the context menu. The new event can be placed
+		wherever the user wishes by right-clicking and choosing 'paste'.
+		"""
+		if eventID >= 0:
+			e = [x for x in self.instrument.graveyard if x.id == eventID][0]
+			self.instrument.graveyard.remove(e)
+		else:
+			e = Event(self.instrument, self.file,  self.levels_file)
+		e.name = self.name
+		
+		dur = self.selection[1] - self.selection[0]
+		
+		e.start = self.start + self.selection[0]
+		e.offset = self.selection[0] #+self.offset
+		e.duration = dur
+		
+		dictLeft = {}
+		dictRight = {}
+		for key, value in self.__fadePointsDict.iteritems():
+			if key < self.selection[0]:
+				dictLeft[key] = value
+			if key > self.selection[0]:
+				dictRight[key - self.selection[0]] = value
+		#in case there is a fade passing through the split point, recreate half of it on either side
+		splitFadeLevel = self.GetFadeLevelAtPoint(self.selection[0])
+		dictLeft[self.selection[0]] = splitFadeLevel
+		dictRight[0.0] = splitFadeLevel
+		
+		millis = int(self.selection[0] * 1000)
+		e.levels_list = self.levels_list.slice_by_endtime(millis)
+		e.__fadePointsDict = dictRight
+			
+		e.__UpdateAudioFadePoints()
+		e.SetProperties()
+		self.instrument.events.append(e)
+		e.emit("length")
+		e.emit("position")
+		
+		return e
+		
+
+	
 	@UndoSystem.UndoCommand("JoinEvent", "temp", "temp2")
 	def SplitEvent(self, split_point, cutRightSide=True, eventID=-1):
 		"""
@@ -1040,31 +1089,42 @@
 		self.fadeLevels = LevelsList.LevelsList()
 		
 		iterFadePoints = iter(self.audioFadePoints)
-		firstFadeTime, firstFadeValue = iterFadePoints.next()
-		firstFadeTime = int(firstFadeTime * 1000)	#convert to milliseconds
-		secondFadeTime, secondFadeValue = iterFadePoints.next()
-		secondFadeTime = int(secondFadeTime * 1000)	#convert to milliseconds
+		try:
+			firstFadeTime, firstFadeValue = iterFadePoints.next()
+			firstFadeTime = int(firstFadeTime * 1000)	#convert to milliseconds
+		except StopIteration:
+			pass
+			
+		try:
+			secondFadeTime, secondFadeValue = iterFadePoints.next()
+			secondFadeTime = int(secondFadeTime * 1000)	#convert to milliseconds
+		except StopIteration:
+			pass
+			
 		# if less than one percent difference, assume they are the same
 		sameValues = abs(firstFadeValue - secondFadeValue) < 0.01
-		
+
 		for endtime, peak in self.levels_list:
 			# check if we have moved into the next fade point pair
 			if (endtime - secondFadeTime) > 1:  # don't care about 1 millisecond difference, its rounding error
 				firstFadeTime = secondFadeTime
 				firstFadeValue = secondFadeValue
-				secondFadeTime, secondFadeValue = iterFadePoints.next()
-				secondFadeTime = int(secondFadeTime * 1000)	#convert to milliseconds
-				
+				try:
+					secondFadeTime, secondFadeValue = iterFadePoints.next()
+					secondFadeTime = int(secondFadeTime * 1000)	#convert to milliseconds
+				except StopIteration:
+					pass #Anything else is pointless
+			
 				# if less than one percent difference, assume they are the same
 				sameValues = abs(firstFadeValue - secondFadeValue) < 0.01
 				if not sameValues:
 					# the fade line is not flat, so calculate the slope of it
 					slope = (secondFadeValue - firstFadeValue) / (secondFadeTime - firstFadeTime)
-			
 			if sameValues:
 				#no fade here, the same volume continues across
 				self.fadeLevels.append(endtime, [int(peak * firstFadeValue)])
 			else:
+				slope = (secondFadeValue - firstFadeValue) / (secondFadeTime - firstFadeTime)
 				rel_time = endtime - firstFadeTime
 				peak_delta = slope * rel_time
 				new_fade_value = firstFadeValue + peak_delta
@@ -1072,7 +1132,6 @@
 				peak = int(peak * new_fade_value)
 				
 				self.fadeLevels.append(endtime, [peak])
-		
 	#_____________________________________________________________________
 	
 	def GetFadeLevels(self):