-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[WIP] Open last active project. Fixes #503 #2140
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
Conversation
|
Sigh, not quite done yet... |
|
Is the team ok with this on by default? This seems a bit drastic over the current functionality, as if a track has an issue (plugin hangs LMMS), the only way to disable this would be to hand-edit (or delete) the -Tres |
|
@tresf this should definitely be default-disabled. At least in master, my
|
|
Oki! ...turning off! |
src/core/main.cpp
Outdated
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.
If you want, the above could be changed to something like:
// If enabled, open last project if there is one. Else, create
// a new one.
if( !ConfigManager::inst()->value( "app", "disableopenlast" ).toInt() &&
!ConfigManager::inst()->recentlyOpenedProjects().isEmpty()) )
{
Engine::getSong()->loadProject(
ConfigManager::inst()->
recentlyOpenedProjects().first() );
}
else
{
Engine::getSong()->createNewProject();
}to avoid the repeat call to Engine::getSong()->createNewProject().
|
Looks fine 👍 I would suggest a description that''s a bit more clear than "Re-open last project" though. "Re-open last project on start" seems like a reasonable improvement, or maybe you can come up without something more descriptive. |
|
Almost there. I incorporated the changes you suggested and inverted the logic. |
|
Yes you can push it here, you'll just need to force push ( |
f0fe4d7 to
7bf591f
Compare
|
OK, pushed!
Which is precisely what I ended up doing a whole lot. Disabled and pushed "eff". ;-) Edited2! Rant removed... |
|
I've tried it some more now and it works just fine.
With the cases I've seen, when LMMS fails to open projects they don't make it to the 'recentlyopened' list so it doesn't try to reopen them next time. |
|
I think It's better to pass a default project for the cases where the most recent file can't be opened. ( has been deleted or renemed... ). As it is now you get an empty project with the title of the project you tried to open. |
|
So this looks good to merge, but unless any other devs disagree with the following, that probably won't happen until after 1.2 is released - we are on a feature-freeze and I would classify this as a new feature. |
|
@Wallacoloo I understand very well the problem to add a new feature while in feature-freeze but IMHO this is a real plus for Lmms in term of user friendliness. |
|
I vote merge. :) |
|
@midi-pascal @tresf Well it takes the meaning of "feature-freeze" away from "feature-freeze", but I don't object - I think we are still a ways away from even having a release candidate ready anyhow, so we'll have plenty of time to correct any small bugs this might introduce. I'll let you do the merge though @tresf so that I don't risk getting yelled at :P |
Agreed. I typed up a justification and then deleted it all. 👍 |
|
I think this PR would be a great addition in 1.2 too :) |
|
I'm cool with it as it is now. I've been thinking back and forth whether it would be good with some kind of info if the last project can't be opened with some explanation for possible reasons but that would probably just be annoying and over doing it. I would like to squash it to one commit though. I like squash. |
Zucchini, yellow, acorn, butternut, pumpkin or anything belonging to genus cucurbita? |
|
@tresf @zonkmachine squash: Nice exercise too but not this easy for beginners with git 😄 |
4b5eed7 to
3972ebb
Compare
Zucchini man, like totally groovy! |
|
I can't re-trigger the failing build. Looks like only team members can do that. Word goes that you can restart the build by closing and opening the PR. Testing... ;P |
|
;-) - I cans hack da Traviz! |
3972ebb to
2aef279
Compare
|
Fixup! |
|
This compiles just fine with #2176 but hasn't been properly tested with that code so I'm marking this WIP. |
|
Merging this now. @zonkmachine you can test against #2176 when you fast-forward that particular branch. |
[WIP] Open last active project. Fixes #503
Issue #503, #1694
This should take care of issue #503 and implement automatic open of the last project. It has an option in 'Settings' to switch it off but is on by default.