Add cli convert subcommand, from raw KLE to JSON #6898

cfbender · 2019-10-04T18:35:15Z

Description

Adds a convert subcommand to convert raw KLE data files to configurator JSON.

Types of Changes

Issues Fixed or Closed by This PR

Resolves CLI command to convert KLE to JSON #6880

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

lib/python/qmk/cli/__init__.py

noroadsleft · 2019-10-07T18:55:52Z

I get an error when I try to run this.

 noroadsleft  /d/GitHub/qmk_firmware/lib/python/qmk/tests (pr/6898)
$ qmk kle2qmk ./kle.txt
<class 'AttributeError'>
☒ 'KLE2xy' object has no attribute 'args'
Traceback (most recent call last):
  File "/d/GitHub/qmk_firmware/lib/python/milc.py", line 564, in __call__
    return self.__call__()
  File "/d/GitHub/qmk_firmware/lib/python/milc.py", line 569, in __call__
    return self._entrypoint(self)
  File "/d/GitHub/qmk_firmware/lib/python/qmk/cli/kle2qmk.py", line 76, in kle2qmk
    layout = json.dumps(kle2qmk(kle), separators=(', ', ':'), cls=CustomJSONEncoder)
  File "/d/GitHub/qmk_firmware/lib/python/qmk/cli/kle2qmk.py", line 33, in kle2qmk
    if cli.args.filename:
AttributeError: 'KLE2xy' object has no attribute 'args'

cfbender · 2019-10-07T22:54:09Z

Great catch, @noroadsleft! I think it was because of two functions both being named kle2qmk (in the converter module and the subcommand name). I renamed it to kle2json, hopefully that is a suitable name @skullydazed!

cfbender · 2019-10-07T23:01:55Z

Hmmm I just did a rebase since it was giving me a merge conflict on lib/python/qmk/cli/__init__.py with the new list command being added. Was that correct? Seems like it changed a lot about this PR

noroadsleft · 2019-10-08T01:38:31Z

@cfbender It looks like you had two different branches with the same set of changes, and then merged one into the other.

Try running git reset --hard 54354250caa53574d82dc097081079722d80f83e locally and seeing if you get to a clean state from which you can pick this back up.

cfbender · 2019-10-08T02:04:52Z

@noroadsleft that was weird, thanks for the help! Looks like it should be all set now.

docs/cli.md

cfbender · 2019-10-08T04:20:19Z

Good catch again @noroadsleft, thank you!

noroadsleft

Just some capitalization fixes, because I'm case-sensitive. 😃

docs/cli.md

lib/python/qmk/cli/kle2json.py

lib/python/qmk/converter.py

lib/python/qmk/cli/kle2json.py

cfbender · 2019-10-08T16:57:04Z

Thanks for the great suggestions @noroadsleft and @skullydazed! I have learned a lot. Hopefully those commits cover it!

noroadsleft

Looks good to me.

drashna

__attribute__((weak)) ✅

skullydazed · 2019-11-13T04:56:04Z

Thanks for your work here, and your patience on the review!

cfbender · 2019-11-13T06:10:47Z

Awesome, thanks @skullydazed! Happy to help and hope to more in the future!

* Add initial pass at KLE convert * Add cli log on convert * Move kle2xy, add absolute filepath arg support * Add overwrite flag, and context sensitive conversion * Update docs/cli.md * Fix converter.py typo * Add convert unit test * Rename to kle2qmk * Rename subcommand * Rename subcommand to kle2json * Change tests to cover rename * Rename in __init__.py * Update CLI docs with new subcommand name * Fix from suggestions in PR qmk#6898 * Help with cases of case sensitivity * Update cli.md * Use angle brackets to indicate required option * Make the output text more accurate

drashna requested review from skullydazed, noroadsleft and a team October 5, 2019 19:05

noroadsleft added cli qmk cli command documentation python labels Oct 5, 2019

skullydazed reviewed Oct 6, 2019

View reviewed changes

lib/python/qmk/cli/__init__.py Outdated Show resolved Hide resolved

skullydazed approved these changes Oct 6, 2019

View reviewed changes

skullydazed requested a review from a team October 6, 2019 21:13

cfbender added 11 commits October 7, 2019 16:56

Add initial pass at KLE convert

840f216

Add cli log on convert

28dac30

Move kle2xy, add absolute filepath arg support

825bd31

Add overwrite flag, and context sensitive conversion

edb7c52

Update docs/cli.md

fe7c625

Fix converter.py typo

00122a3

Add convert unit test

a7a675f

Rename to kle2qmk

ae5df8d

Rename subcommand

23a278f

Rename subcommand to kle2json

0ab880d

Change tests to cover rename

5435425

cfbender force-pushed the cli-convert branch from e6943cd to 5435425 Compare October 8, 2019 02:01

Rename in __init__.py

45617d7

noroadsleft requested changes Oct 8, 2019

View reviewed changes

docs/cli.md Outdated Show resolved Hide resolved

docs/cli.md Outdated Show resolved Hide resolved

docs/cli.md Outdated Show resolved Hide resolved

docs/cli.md Outdated Show resolved Hide resolved

docs/cli.md Outdated Show resolved Hide resolved

Update CLI docs with new subcommand name

2ef5baa

noroadsleft requested changes Oct 8, 2019

View reviewed changes

docs/cli.md Outdated Show resolved Hide resolved

lib/python/qmk/cli/kle2json.py Outdated Show resolved Hide resolved

lib/python/qmk/converter.py Outdated Show resolved Hide resolved

lib/python/qmk/converter.py Outdated Show resolved Hide resolved

skullydazed reviewed Oct 8, 2019

View reviewed changes

cfbender added 2 commits October 8, 2019 10:55

Fix from suggestions in PR qmk#6898

95315c1

Help with cases of case sensitivity

e3400b7

noroadsleft requested a review from skullydazed October 8, 2019 17:24

noroadsleft approved these changes Oct 8, 2019

View reviewed changes

drashna approved these changes Oct 8, 2019

View reviewed changes

skullydazed added 3 commits November 12, 2019 20:50

Update cli.md

30f3b91

Use angle brackets to indicate required option

09d8faa

Make the output text more accurate

0f0f339

skullydazed approved these changes Nov 13, 2019

View reviewed changes

Merge branch 'master' into cli-convert

180e595

skullydazed merged commit 7329c2d into qmk:master Nov 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cli convert subcommand, from raw KLE to JSON #6898

Add cli convert subcommand, from raw KLE to JSON #6898

cfbender commented Oct 4, 2019

noroadsleft commented Oct 7, 2019

cfbender commented Oct 7, 2019

cfbender commented Oct 7, 2019

noroadsleft commented Oct 8, 2019

cfbender commented Oct 8, 2019

cfbender commented Oct 8, 2019

noroadsleft left a comment

cfbender commented Oct 8, 2019

noroadsleft left a comment

drashna left a comment

skullydazed commented Nov 13, 2019

cfbender commented Nov 13, 2019

Add cli convert subcommand, from raw KLE to JSON #6898

Add cli convert subcommand, from raw KLE to JSON #6898

Conversation

cfbender commented Oct 4, 2019

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

noroadsleft commented Oct 7, 2019

cfbender commented Oct 7, 2019

cfbender commented Oct 7, 2019

noroadsleft commented Oct 8, 2019

cfbender commented Oct 8, 2019

cfbender commented Oct 8, 2019

noroadsleft left a comment

Choose a reason for hiding this comment

cfbender commented Oct 8, 2019

noroadsleft left a comment

Choose a reason for hiding this comment

drashna left a comment

Choose a reason for hiding this comment

skullydazed commented Nov 13, 2019

cfbender commented Nov 13, 2019