search

caksoylar / keymap-drawer

Visualize keymaps that use advanced features like hold-taps and combos, with automatic parsing
https://caksoylar.github.io/keymap-drawer
MIT License
730 stars 62 forks source link

Add path argument for <keymap>.json layout file #71

Closed michaelrommel closed 9 months ago

michaelrommel commented 9 months ago

Hi Cem,

I have created a small patch to make it possible that the path to the layout file for a keyboard's keymap can be specified in the arguments to the draw-zmk.yaml workflow.

Unfortunately my linter automatically reformatted all double quotes... I am sorry about that, just saw that. Also I was unsure, whether you wanted perhaps a different implementation, like not a directory, but a full path to the .json file or so. Let me know, if I should change the PR.

Thanks a lot for the drawer - I hate it, when I have to manually generate artwork, whenever a technical document changes...

BTW: the repo where it is used is here: https://github.com/michaelrommel/zmk-config


├── README.md
├── build.yaml
├── config
│   ├── boards
│   │   └── arm
│   │       └── nightliner
│   │           ├── Kconfig
│   │           ├── Kconfig.board
│   │           ├── Kconfig.defconfig
│   │           ├── board.cmake
│   │           ├── nightliner-pinctrl.dtsi
│   │           ├── nightliner.dtsi
│   │           ├── nightliner.keymap
│   │           ├── nightliner.yaml
│   │           ├── nightliner.zmk.yml
│   │           ├── nightliner_left.dts
│   │           ├── nightliner_left_defconfig
│   │           ├── nightliner_right.dts
│   │           └── nightliner_right_defconfig
│   └── west.yml
├── images
│   ├── keymap_drawer.config.yaml
│   ├── nightliner.json
│   ├── nightliner.png
│   ├── nightliner.svg
│   ├── nightliner.yaml
│   ├── nightliner_keypositions.png
│   └── nightliner_prev.yaml
├── scripts
│   └── create_svg.sh
├── tags
└── zephyr
    └── module.yml
michaelrommel commented 9 months ago

I understand and agree, have applied the changes, this could be finalized now, I did two checks with an invalid and a missing path, both fell back to trying to draw without the json layout spec.

On more question, though: In testing I now understood, that a subsequent job in the workflow, even if it "needs" the draw step, cannot access the new .svg file, because there is a delay between the amending commit and the file actually being available for the next checkout. I am new to GH workflows, so please forgive any newbie knowledge gap... In researching how to resolve this, I understood, that only using "artifacts" may help in this case to make a non-textual output of a workflow available to other jobs.

My question: does it make sense to you to add functionality to decide whether the svg file shall be uploaded in addition to the commit or as alternative to a commit to an artifact? I would either do a separate PR if you are interested. I need it, because my subsequent step is to render the svg to a png via an inkscape cli call.

Thanks again,

Michael.

michaelrommel commented 9 months ago

Hm, I seem to have mistakenly closed the PR. Can you still merge? Sorry about this...

michaelrommel commented 9 months ago

Hi Cem,

it's me again. I think I have now figured all out. What I wanted to achieve is to create an svg and a converted png in one workflow. So I changed your workflow again to make it further configurable and decide whether to directly commit the .yaml and .svg files or to upload them to an artifact.

In my calling workflow, I now call this reusable workflow, then check out my zmk-config repo, download the artifacts, do the conversion to png and then commit all three files per keyboard back to the repo.

I have not created a new PR, since I was unsure, if you wanted to include all this in your template. Maybe it's overkill for most people... Anyway the new wf lives here: https://github.com/michaelrommel/keymap-drawer/blob/artifactupload/.github/workflows/draw-zmk.yml in a new branch.

And I use it like this here: https://github.com/michaelrommel/zmk-config/actions/runs/7906653515

It would be nice, if you let me know, if you are interested or if I should repackage it into my own repo of GH reusable workflows.

Thanks and have a nice day!

Michael.

caksoylar commented 9 months ago

Thank you for testing and confirming, and apologies for the last description tweak I made that I hadn't caught before.

I think adding an upload artifact option is a good idea, and I like how you implemented it with the destination argument. So I'd definitely appreciate a PR! It would also help solve #58 I believe.