Skip to content

Conversation

fschuelke
Copy link
Contributor

commit for #5263

@PhysSong
Copy link
Member

You should use tabs for indentation instead of spaces.
https://github.com/LMMS/lmms/wiki/Coding-Conventions#indentation

@LmmsBot
Copy link

LmmsBot commented Oct 26, 2019

🤖 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

macOS

Windows

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://13334-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.97%2Bgca2cca9-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13334?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://13332-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.97%2Bgca2cca911-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13332?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://13335-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.97%2Bgca2cca911-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13335?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://13331-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.97%2Bgca2cca911-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13331?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/p3ssvlp8418bsxyl/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38538470"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/wwocd1sjpnc3g69t/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38538470"}]}, "commit_sha": "726ecc9bb52491a3bae46b38f4433ded83ec91f1"}

@fschuelke
Copy link
Contributor Author

You should use tabs for indentation instead of spaces.
https://github.com/LMMS/lmms/wiki/Coding-Conventions#indentation

I did use tabs, could be the issue of the IDE... do we have a clang-tidy format or anything like that? the code shows up normally formatted for me

@fschuelke
Copy link
Contributor Author

@PhysSong i checked the indentation, but for me it looks fine. Also doesnt show any wrong indentation in the git file viewer either

@PhysSong
Copy link
Member

PhysSong commented Nov 1, 2019

If there was a tab instead of 4 spaces, this wouldn't be possible.
image

@fschuelke
Copy link
Contributor Author

If there was a tab instead of 4 spaces, this wouldn't be possible.
image

im gonna commit it directly in github. dont know why clion shows me correct indentation when there isnt

m_trackContainerView->removeTrackView( this );
return QWidget::close();
m_trackContainerView->removeTrackView( this );
return QWidget::close();
Copy link
Member

Choose a reason for hiding this comment

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

Spaces -> tabs

@Umcaruje
Copy link
Member

Umcaruje commented Nov 22, 2019 via email

@PhysSong
Copy link
Member

The user should be simply able to actually undo their track deletion.

We should allow that finally. However, I think it needs much more code changes than what are in this PR.

@fschuelke
Copy link
Contributor Author

/usr/local/bin/appdmg -> /usr/local/lib/node_modules/appdmg/bin/appdmg.js

macos-alias@0.2.11 install /usr/local/lib/node_modules/appdmg/node_modules/macos-alias
node-gyp rebuild

ERROR:root:code for hash md5 was not found.

First lines of the circleci calls; is the error caused by any programming?

@PhysSong
Copy link
Member

PhysSong commented Dec 6, 2019

Ignore the error, it's not related to this PR. You can see the same message in many other places.

@PhysSong
Copy link
Member

I think this is not the right approach to tackle this problem.

@Umcaruje Then, what do you want for this PR?

@zonkmachine
Copy link
Contributor

What's the status of this? Whitespace changes can be done by any of us.

@PhysSong
Copy link
Member

I resolved merge conflicts and fixed formatting issues.

PhysSong and others added 4 commits December 19, 2020 12:27
Update Hombebrew manually to avoid errors with shallow clones.
Co-authored-by: PhysSong <tteu.ingog@gmail.com>
* Fixes bug with pasting of TCOs (#5840)

	TimePos::quantize works for negative values, but ends
up snapping the TCO to the opposite direction. This is because the
snapping happens in the direction of the origin, which is left for
positive values and right for negative values.
	That wasn't accounted for in the pasteSelection method
and we ended up with wrong positions when pasting before the
TCO(s) we copied.
	This PR fixes the issue by ensuring that we snap in the same direction when halfway through an interval, regardless of negative or positive offset.

* Fixes a calculation on TimePos::quantize

	Since we are working with integers, using "offset /
(interval/2)" would be problematic if interval was odd. We instead
multiply both sides by two and use "(2 * offset) / interval" to obtain
the result for snapUp.
@PhysSong
Copy link
Member

There was a suggestion from Discord: there should be a way for users to suppress the dialog until the proper undo/redo function is implemented.

deenadayalancs and others added 21 commits January 23, 2021 12:09
Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
* Initial PianoRoll razor feature

* Restore PianoRoll edit mode after focusOut and in razor mode.

* Show changes directly after cut.

* Fix hanging note after adjusting vol/pan with razor action.

* Extract the split action to a separate method

	This PR addresses some suggestions from a review, the most
important ones being:
	- Extracting the note split action to a separate method, called
Pattern::splitNotes
	- Removing getMouseTickPos method
	- Adding a variable that holds the current razor position and a
method to update it (quantizing if CTRL is not pressed)
	- Using [this] to capture "this" on the lambda function instead
of [=], since the latter doesn't work as intended from C++20 forward
	- Fixing some code style and adding comments

* Removes an extra call to noteUnderMouse

	By removing "&& noteUnderMouse()" from the mousePressEvent
conditional, we avoid an extra call to noteUnderMouse. The only
difference in the behavior of the tool is that now clicking on a place
that doesn't have a note will exit Razor mode.

* Style change suggested by @russiankumar

* Cancel razor action on SHIFT release.

* Make razor cut-line (color) themable.

* Add razor cut-line color to classic theme style.css

* Rename razor to knife.

* Change pixmap from razor to knife (from #5524)

* Remove SHIFT behavior.

* Change knife shortcut to SHIFT+K

Co-authored-by: CYBERDEViL <cyberdevil@notabug.org>
Co-authored-by: Ian Caio <iancaio_dev@hotmail.com>
* Add new note length modification tools

* "Last" instead of "latest"

* Code formatting

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Fix sort

* Fix incorrect cut of last note

* Fix Fill not stretching last note to end

* Fill from end positions

* Rename limitNoteLengths

* Compare with non-selected notes when filling

* Style changes by IanCaio + bugfix

* break instead of continue

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>
* pianoroll: nudge/snap while dragging notes

combobox for switching behavior
old behavior (nudge) is default

* snap note start or note end to nearest grid line

* fix member variable name

* change default width of pianoroll

* remove trailing white space

* use mouse position instead of m_draggednote

* Uses enum instead of text for GridMode selection

	To avoid problems with translations breaking the conditional,
PianoRoll::changeSnapMode now uses the value of the ComboBox model,
casted to the GridMode enum instead of the text.

* Fixes last code style issues

	A few code style issues were left, those are fixed in this
commit.

* Removes forgotten comma on enum

	Since the last enum value was commented out, there's an
unnecessary comma left that was removed.

* Rewrites some of the grid logic

	Separates Nudge logic and Snap logic more explicitly.
	Avoids recalculation of mouse conversion to time line ticks.
	Disables shift resizing for Snap mode.
	Removes unnecessary range checks.
	Fixes bug with note key boundary checks (which is present in master).

* Make noteOffset an int instead of TimePos

Co-authored-by: Ian Caio <iancaio_dev@hotmail.com>
m_gridMode was not being initialized on the PianoRoll
constructor. For that reason the first note drag without changing the
GridMode would result in m_gridMode being used unintialized and neither
the Snap nor Nudge mode being used. Here, the result of this was a note
being moved unquantized.
	This fixes it by calling changeSnapMode() after setting up the
snapModel on the constructor.
* Add Compressor effect
Prevents a segfault when attempting to copy construct an automation pattern with no track. Thanks to SeleDreams, Veratil, Dom.
* Rebase BaraMGB's Knife
Co-authored-by: Steffen Baranowsky <BaraMGB@freenet.de>

* Draw marker

* Refactoring and shift mode

* Allow resizing

* Add Icon

* Fix stuck marker on RMB, remove unnecessary cast

* Remove redundant line, more const

* Fix

* Review fixes

* Only perform split logic for SampleTCO

* Add unquantizedModHeld function

* missed one

* Don't use copy/paste

* Don't use copy/paste

* More git troubles

* Fix undo

* git dammit

* Cleaner solution?

* Set cursor, add copy assignment to SampleBuffer

* Add TODO comment

* Make it build

* Fixes from review

* Make splitTCO virtual

* Make splitTCO more generic

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Prevent resizing of MIDI clips in knife mode

* Fix move/resize and rework box select via ctrl

* Apply suggestions from code review.

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Don't show inaccurate/useless/empty text float in knife mode

* Addresses Github review

	- Fixes a typo where QWidget::mousePressEvent was being called
inside mouseReleaseEvent.
	- Avoids unnecessarily disabling journalling on the Split
action, since it doesn't require it.

* Revert format changes in Track

* Revert format changes in Track.h

* Revert formatting changes in Track.cpp

Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
Co-authored-by: IanCaio <iancaio_dev@hotmail.com>
* Add changes extracted from microtonal PR

* Apply suggestions from code review

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Apply suggestions from code review

(adding oneliner spaces)

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Update src/gui/widgets/LcdFloatSpinBox.cpp

Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>

* Apply suggestions from code review

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>

* Implement suggestions from review

* Reorder constructor parameters, remove old code for cursor movement and hiding from Lcd*SpinBoxes

* Apply suggestions from code review

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>

* Fix right margin width in seamless mode

* Add explanation to border drawing code

Co-authored-by: IanCaio <iancaio_dev@hotmail.com>
Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
* Button with menu has split highlight

* Add menu with more action to quantize button

* Style changes

* Fix CSS length-zero-no-unit warning

* Add combo button to classic theme
@Spekular
Copy link
Member

So I'm struggling to push to this branch for whatever reason, but I've gone ahead and added a setting that lets users opt out of this warning. They can either check a box that says "Don't show again" in the dialogue, or go to Edit > Settings and change it there.

master...Spekular:trackDeletion

@PhysSong
Copy link
Member

@Spekular I can, but you need a default 1 in TrackOperationsWidget::confirmRemoval for needConfirm.

@Spekular
Copy link
Member

@PhysSong good point! I added a default value now.

@Spekular Spekular merged commit 8d9c734 into LMMS:master Apr 3, 2021
devnexen pushed a commit to devnexen/lmms that referenced this pull request Apr 10, 2021
* Added messagebox when user tries to delete track

* Code review refactorings

* Update Track.h

* Changed message box title to "Confirm removal"

* Merge changes from master

* Add option to disable warning

* Default to showing warning if no config found

Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
Co-authored-by: Spekular <Spekular@users.noreply.github.com>
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
* Added messagebox when user tries to delete track

* Code review refactorings

* Update Track.h

* Changed message box title to "Confirm removal"

* Merge changes from master

* Add option to disable warning

* Default to showing warning if no config found

Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
Co-authored-by: Spekular <Spekular@users.noreply.github.com>
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.

Add warning dialogue for "Remove this track" button in song editor.