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

Adding my DZ60, Satan, XD75 kemaps and updates to userspace #4039

Merged
merged 7 commits into from
Oct 2, 2018

Conversation

JarredSteenvoorden
Copy link
Contributor

Adding keymaps for DZ60, Satan and XD75 (mine and a friends).

Also small update to userspace.

Cheers!

@@ -0,0 +1,8 @@
#ifndef CONFIG_USER_H
Copy link
Member

Choose a reason for hiding this comment

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

If you could replace the include guard (the ifndef, define, and endif at the end) with just #pragma once?

It's simpler, and less prone to user error, so if you could make this switch?

Copy link
Member

Choose a reason for hiding this comment

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

@drashna Actually, I think this file can be deleted.

  • Line 4 isn't needed; it's included automatically by the build script.
  • Line 6 isn't needed. It's default behavior.

With your changes, that leaves only #pragma once in this file.

#ifndef CONFIG_USER_H
#define CONFIG_USER_H

#include "../../config.h"
Copy link
Member

Choose a reason for hiding this comment

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

This line is not actually needed, and should be removed.

Specifically, the build script automatically includes all of the config.h files, so this is not necessary.


#include "../../config.h"

#define PREVENT_STUCK_MODIFIERS
Copy link
Member

Choose a reason for hiding this comment

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

This is actually the new default now. This isn't needed. But you may need to update your fork to get the changes.

@@ -19,6 +19,8 @@

#include "config_common.h"

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

You have merge conflicts here, you need to fix this.

@@ -0,0 +1,8 @@
#ifndef CONFIG_USER_H
Copy link
Member

Choose a reason for hiding this comment

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

If you could replace the include guard (the ifndef, define, and endif at the end) with just #pragma once?

It's simpler, and less prone to user error, so if you could make this switch?

Copy link
Member

Choose a reason for hiding this comment

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

This file I believe can be deleted as well, for the same reasons as the config.h file for the DZ60 keymap.

#ifndef CONFIG_USER_H
#define CONFIG_USER_H

#include "../../config.h"
Copy link
Member

Choose a reason for hiding this comment

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

This line is not actually needed, and should be removed.

Specifically, the build script automatically includes all of the config.h files, so this is not necessary.


#include "../../config.h"

#define PREVENT_STUCK_MODIFIERS
Copy link
Member

Choose a reason for hiding this comment

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

This line is not actually needed, and should be removed.

Specifically, the build script automatically includes all of the config.h files, so this is not necessary.

#ifndef USERSPACE_CONFIG_H
#define USERSPACE_CONFIG_H

#define PREVENT_STUCK_MODIFIERS
Copy link
Member

Choose a reason for hiding this comment

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

This line is not actually needed, and should be removed.

Specifically, the build script automatically includes all of the config.h files, so this is not necessary.

* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#ifndef USERSPACE_CONFIG_H
Copy link
Member

Choose a reason for hiding this comment

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

If you could replace the include guard (the ifndef, define, and endif at the end) with just #pragma once?

It's simpler, and less prone to user error, so if you could make this switch?

@@ -0,0 +1,3 @@
ifndef QUANTUM_DIR
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this (ifndef/include/endif) block?

It's not needed, and is a holdover from the old build system.

Copy link
Member

Choose a reason for hiding this comment

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

File can be deleted, nothing going on here.

@drashna drashna added the keymap label Oct 2, 2018
@@ -0,0 +1,24 @@
#include QMK_KEYBOARD_H

#define _______ KC_TRNS
Copy link
Member

Choose a reason for hiding this comment

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

Lines 3 and 4 can be safely deleted. QMK includes these definitions automatically with every keymap.

// Fillers to make layering more clear
#define _______ KC_TRNS
#define XXXXXXX KC_NO

@@ -1,4 +1 @@
# Build options
Copy link
Member

Choose a reason for hiding this comment

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

This file can be deleted; nothing's actually happening here.

@@ -0,0 +1,25 @@
#include QMK_KEYBOARD_H

#define _______ KC_TRNS
Copy link
Member

Choose a reason for hiding this comment

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

Lines 3 and 4 can be safely deleted. QMK includes these definitions automatically with every keymap.

// Fillers to make layering more clear
#define _______ KC_TRNS
#define XXXXXXX KC_NO

# change to "no" to disable the options, or define them in the Makefile in
# the appropriate keymap folder that will get included automatically
#
BOOTMAGIC_ENABLE = no # Virtual DIP switch configuration(+1000)
Copy link
Member

Choose a reason for hiding this comment

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

You can actually compress Lines 5-17 down to:

MOUSEKEY_ENABLE = yes       # Mouse keys(+4700)
CONSOLE_ENABLE = no         # Console for debug(+400)
RGBLIGHT_ENABLE = no        # Enable WS2812 RGB underlight.  Do not enable this with audio at the same time.
SLEEP_LED_ENABLE = no       # Breathing sleep LED during USB suspend

The settings only need to be included here if you're changing them from what is set by the keyboard's rules.mk file (keyboards/satan/rules.mk, in this case).

#define _NV 1
#define _NM 2

#define _______ KC_TRNS
Copy link
Member

Choose a reason for hiding this comment

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

Lines 7 and 8 can be safely deleted. QMK includes these definitions automatically with every keymap.

// Fillers to make layering more clear
#define _______ KC_TRNS
#define XXXXXXX KC_NO

@@ -0,0 +1,8 @@
#ifndef CONFIG_USER_H
Copy link
Member

Choose a reason for hiding this comment

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

This file I think can be deleted, as with the other keymap config.h files, for the same reasons as before.

@@ -0,0 +1,3 @@
ifndef QUANTUM_DIR
Copy link
Member

Choose a reason for hiding this comment

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

This file should be deleted. The code block is unnecessary as @drashna said, and there's nothing else here.

@@ -0,0 +1,8 @@
#ifndef CONFIG_USER_H
Copy link
Member

Choose a reason for hiding this comment

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

@noroadsleft says "Delete this file, blah blah blah, I've heard it a hundred times already."

😀

#define _NV 2
#define _NM 3

#define _______ KC_TRNS
Copy link
Member

Choose a reason for hiding this comment

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

Lines 8 and 9 can be safely deleted. QMK includes these definitions automatically with every keymap.

// Fillers to make layering more clear
#define _______ KC_TRNS
#define XXXXXXX KC_NO

@@ -0,0 +1,3 @@
ifndef QUANTUM_DIR
Copy link
Member

Choose a reason for hiding this comment

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

File can be deleted, nothing going on here.

@JarredSteenvoorden
Copy link
Contributor Author

Wow there was alot of redundant code in these older keymaps, should be cleaned up now :)

enum userspace_custom_keycodes {
VRSN = SAFE_RANGE // Prints QMK Firmware and board info
};

// Use 7 wide characters for keymaps
Copy link
Member

Choose a reason for hiding this comment

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

Missed this before.

Lines 26-28 shouldn't be needed.

But everything else to me looks good.

@noroadsleft
Copy link
Member

Travis CI error is unrelated.

@drashna
Copy link
Member

drashna commented Oct 2, 2018

Thanks!

@drashna drashna merged commit 5c2ac73 into qmk:master Oct 2, 2018
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Oct 13, 2018
…mk#4039)

* Update userspace with common config.h

* Add my dz60, satan and xd75 keyboard keymaps

* Fixing executable bits changed during last upstream merge

* Cleanup unnecessary files and defines

* Remove unnecessary defines from userspace config
rseymour pushed a commit to rseymour/qmk_firmware that referenced this pull request Mar 13, 2019
…mk#4039)

* Update userspace with common config.h

* Add my dz60, satan and xd75 keyboard keymaps

* Fixing executable bits changed during last upstream merge

* Cleanup unnecessary files and defines

* Remove unnecessary defines from userspace config
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
…mk#4039)

* Update userspace with common config.h

* Add my dz60, satan and xd75 keyboard keymaps

* Fixing executable bits changed during last upstream merge

* Cleanup unnecessary files and defines

* Remove unnecessary defines from userspace config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants