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

Devided #3856, split Space Cadet process, trying fix at #3701 #3885

Closed
wants to merge 34 commits into from

Conversation

leico
Copy link
Contributor

@leico leico commented Sep 9, 2018

update: 09/12/18

Todo:

  • NO_SPACE_CADET or SPACE_CADET_ENABLE
    • implement
    • adapt default keymaps
    • review
  • change manuals
    • ESC_GRAVE
    • SPACE_CADET_SHIFT
    • SHIFT_ENTER
    • need review (I'm not English native.)
  • delete old sourced(comment outs)
  • add comments if needed

@drashna
Copy link
Member

drashna commented Sep 10, 2018

I hate to ask....

But while you're doing this, would you mind adding something like NO_SPACE_CADET, so that this code can be completely removed, if desired?

@leico
Copy link
Contributor Author

leico commented Sep 10, 2018

OK, will try it as soon as possible. No ploblem.

Copy link
Member

@skullydazed skullydazed left a comment

Choose a reason for hiding this comment

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

We're still discussing the overall structure internally. In the meantime please see my comments above for changes we know you can make now.

docs/feature_grave_esc.md Outdated Show resolved Hide resolved
docs/feature_grave_esc.md Outdated Show resolved Hide resolved
docs/feature_grave_esc.md Outdated Show resolved Hide resolved
docs/feature_grave_esc.md Outdated Show resolved Hide resolved
docs/feature_space_cadet_shift.md Show resolved Hide resolved
keyboards/alu84/keymaps/default/rules.mk Outdated Show resolved Hide resolved
quantum/quantum.c Outdated Show resolved Hide resolved
quantum/quantum_keycodes.h Outdated Show resolved Hide resolved
revert docs/feature_grave_escape.md from before change grave_escape process
delete space between table symbol |
revert quantum/quantum_keycodes.h
@leico
Copy link
Contributor Author

leico commented Sep 12, 2018

interim pull request...

  • revert grave escape
    • doc
    • quantum.c
  • improve docs
    • space cadet shift
  • issue of enable default or disable default
    • change keymaps setting (if needed)
      • [keyboard]/default keymap
      • layouts directory
      • disable default on keyboard template

@leico leico changed the title Devided #3856, split Space Cadet process, trying fix at #3701 and #3769 Devided #3856, split Space Cadet process, trying fix at #3701 Sep 13, 2018
conflicts in common_features.mk

master      adds dynamic_keymap
space_cadet adds space_cadet

both leave in common_features.mk
… templates

* rules.mk
	add SPACE_CADET_ENABLE = no
	commented around +520 bytes increase

* documents
	* feature_space_cadet_shift.md
	* feature_space_cadet_shift_enter.md
		noted around +520 bytes increase
@leico
Copy link
Contributor Author

leico commented Oct 23, 2018

@drashna , Thank you reply.

Perhaps all of space cadet used layouts would enable, also template goes disable default.
one more, compare binary size between enable and disable, noted in documents how many bytes increase when enable.

@drashna
Copy link
Member

drashna commented Feb 28, 2019

This PR and #5226 aim to do the same thing. Though, that PR includes some additional functionality.

@leico
Copy link
Contributor Author

leico commented Feb 28, 2019

@drashna yes, It looks similar directionality enhancements, #5226 looks more multifunctional. I am still waiting a result about internal structure discussion. 4 month ago, I and @skullydazed afraid to QMK souces become a #define , #ifdef spagetti hell.

@drashna
Copy link
Member

drashna commented Jun 6, 2019

I hate to say it, but I think that the space cadet feature has been overhauled, and also meets most of the same goals that this PR had, plus more.

That's why there are a bunch of merge conflicts now.

I'm closing this for now. If you don't feel that's correct/appropriate, then please do re-open, and update your PR.

@drashna drashna closed this Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants