Skip to content
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

Add insert/remove bar buttons in Song editor #5698

Merged
merged 1 commit into from
Oct 4, 2020

Conversation

allejok96
Copy link
Contributor

Added two buttons for the shortcuts insert and remove bar #5602

This is my first ever pull request and piece of C++ code, so forgive any errors.

bild

@LmmsBot
Copy link

LmmsBot commented Oct 3, 2020

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://9272-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-720%2Bg86c1ba4-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/9272?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://9273-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-720%2Bg86c1ba43f-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/9273?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://9270-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-720%2Bg86c1ba43f-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/9270?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/7832xx4k2sv73yx6/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/35553307"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/f90m3jn0vmung77f/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/35553307"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://9271-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-720%2Bg86c1ba43f-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/9271?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "9263a21cc4c9be7bb76f8260be8daca9df3a3cfd"}

@JohannesLorenz JohannesLorenz self-requested a review October 4, 2020 12:50
@JohannesLorenz
Copy link
Contributor

JohannesLorenz commented Oct 4, 2020

@allejok96 I get an error while starting LMMS:

Error loading icon pixmap "insert_bar": File not found
Error loading icon pixmap "remove_bar": File not found

Indeed, I see not pixmaps on the new buttons. Did you forget to add them to your commit?

Edit: I see the pixmaps are in default theme, which I have selected ✔️ However, they are not installed yet.

@allejok96
Copy link
Contributor Author

allejok96 commented Oct 4, 2020

@JohannesLorenz Will the error fix itself or do I need to change anything?
And does classic theme need the icons too? Ah, it grabs them from default if they are missing...

@JohannesLorenz
Copy link
Contributor

Will the error fix itself

Indeed! I had to do a make clean and a further make install to get it run. Seems like a bug of the build system.

Now going on with the review...

Copy link
Contributor

@JohannesLorenz JohannesLorenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JohannesLorenz JohannesLorenz merged commit 8939b14 into LMMS:master Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants