-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Editor Refactoring #1426
Merged
Editor Refactoring #1426
Changes from 13 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
44b52eb
Add Editor superclass
lukas-w c6ee614
Clean up ToolButton class
lukas-w 02006f9
Use Editor superclass in AutomationEditor
lukas-w 3e9cc61
SongEditor: Use Editor superclass
lukas-w 86f2c86
Move play, record and stop signals to superclass
lukas-w d8db894
Editor: Don't delete on close
lukas-w ec9158c
PianoRoll: Use Editor superclass
lukas-w e9d841d
Migrate Timeline::addToolButtons to QToolBar
lukas-w 4b27569
ToolBar css fixes
lukas-w d029c85
BBEditor: Use Editor superclass
lukas-w 11898a5
Move Timeline.cpp to gui directory
lukas-w 7a21d69
SongEditor: Some renames
lukas-w f131fbd
Editors: Add to workspace in MainWindow class, not in themselves
lukas-w 32da8cb
Editor: Don't use ToolButton
lukas-w 968e581
Editors: Don't use ToolButton
lukas-w b661e08
PianoRoll: Slot renames
lukas-w 47cbc9e
AutomationEditor + PianoRoll: Move Copy/Paste shortcuts
lukas-w 409e8f2
AutomationEditor style updates
lukas-w 51f5929
Rename Timeline to TimeLineWidget
lukas-w b25765d
Move Editors to src/gui/editors subdirectory
lukas-w 7c508f7
Merge master into ed_refac
lukas-w 1d07a91
PianoRoll: Coding style updates
lukas-w ebbec2f
Editor: Add edit mode support
lukas-w 9b6612c
PianoRoll rename fix
lukas-w 02869b1
Editors: Some cleanups
lukas-w 7877888
Introduce ActionGroup subclass
lukas-w 11cb8b5
Automation Editor tension fix
lukas-w 657fb06
More Automation refactoring
lukas-w eb79701
Merge branch 'master' into ed_refac
lukas-w 1ee9340
Move Engine' GUI code to new GuiApplication class
lukas-w 0df3998
Move some gui initialization to GuiApplication's constructor
lukas-w 834be94
Merge commit 'f7741f184f83e6b9e2f081d39efffb2c499962f6' into ed_refac
lukas-w 1706279
Merge commit '25ab7260f5cc57075360c976826e13434ade058c' into ed_refac
lukas-w 0680669
Merge commit 'b5538c7da818cbcdde5ff1c885ce4eee5b626f3b' into ed_refac
lukas-w 23e0e0f
Merge branch 'master' into ed_refac
lukas-w 0c4833c
Adjust automation editor flip implementation
lukas-w 748cccd
Merge branch 'gui_application' into ed_refac
lukas-w e0dbfa6
Remove Engine's has_gui option
lukas-w 7f2f9f2
Merge branch 'master' into ed_refac
lukas-w 23dbe95
Stop on second space key press
lukas-w 56055b3
Merge branch 'master' into ed_refac
lukas-w File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think our convention is to use CapitalCase for enum members...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't invent that enum, I just moved it to public ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I know, just thought that since you're already refactoring things,
you wouldn't mind some extra work ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll do some style updates sooner or later ;) I think you renamed that particular enum in
lmms2
already.Btw, I suggest dropping the convention about parentheses being followed/preceded by a space character (like
printf( "Hello World!" );
instead ofprintf("Hello World!");
). It can get really hard to stick to, and it doesn't really add much to readability.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 12/08/2014 09:55 PM, Lukas W wrote:
Did I? Can't remember if I've touched AutomationEditor in lmms2 yet...
Sorry but personally, I have to disagree with that. I actually think it
clears up the code a lot... it adds some separation between arguments
and it makes it a whole lot easier to read especially when there are a
lot of nested function calls. Or complex equations with lots of
parentheses. For example, compare this:
to this:
The first one looks a whole lot more readable to me...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never suggested removing spaces between operators.