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

Feature: transform the display #43

Closed
wants to merge 11 commits into from

Conversation

alex-courtis
Copy link
Owner

@alex-courtis alex-courtis commented Jun 6, 2022

Closes #41

@anandubey let's continue discussions in here.

Copy link
Owner Author

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

This is looking good and the 180 flip is working.

TODO

  • fixes mentioned in review
  • Actually apply the Cfg value in desire_transform
  • Print the transformation information in info.c
  • Add transformation to CLI; merge etc. looks complete so I expect it should Just Work

src/cfg.cpp Outdated Show resolved Hide resolved
src/cfg.cpp Outdated
cfg_user_mode_free(user_transform);
continue;
}
if (!parse_node_val_int(transform, "DEGREE", &user_transform->transform, "TRANSFORM", user_transform->name_desc)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

We will need to be more strict about the value here: only a few values are applicable, and we need to account for flipped.

We can use a similar scheme to sway:

Can be one of "90", "180", "270" for rotation; or "FLIPPED", "FLIPPED-90", "FLIPPED-180", "FLIPPED-270" to apply a rotation and flip, or "NORMAL" to apply no transform.

convert.c methods similar to auto_scale_val and auto_scale_name will need to be created to perform that validation and conversion.

Choose a reason for hiding this comment

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

Need time to figure out this one. Not getting enough time from work :(

Copy link
Owner Author

Choose a reason for hiding this comment

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

There is no rush

inc/cfg.h Outdated Show resolved Hide resolved
src/cfg.cpp Outdated Show resolved Hide resolved
src/head.c Outdated Show resolved Hide resolved
src/layout.c Show resolved Hide resolved
- ability to specify transform value in terms of int
- ability to hot reload
- prints info value with transform info
- accepts transform through cmd line (does not work - still broken)
@anandubey
Copy link

anandubey commented Jul 4, 2022

Things left to do:

  • cmd line does not work properly.
  • flipped transform was a bit buggy in my testing.

Thank you for waiting. Last month, I was super occupied at work.

Copy link
Owner Author

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Made a few fixes. I don't have access you push to your repo, so here are some patches you can git apply ... in order.
0001-formatting.patch.txt
0002-remove-head.tranform-always-use-desired-and-current.patch.txt
0003-assorted-fixes-for-transform-read-write.patch.txt

This is working quite nicely now.

Tested OK:

  • value from cfg.yaml
  • set / reset from CLI
  • delete from CLI
  • write from CLI

TODO:

src/cfg.cpp Outdated Show resolved Hide resolved
src/cfg.cpp Outdated Show resolved Hide resolved
src/cfg.cpp Outdated
cfg_user_mode_free(user_transform);
continue;
}
if (!parse_node_val_int(transform, "DEGREE", &user_transform->transform, "TRANSFORM", user_transform->name_desc)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

There is no rush

src/layout.c Show resolved Hide resolved
src/layout.c Outdated
if (!head->desired.enabled)
return;

// attempt to find a mode
Copy link
Owner Author

Choose a reason for hiding this comment

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

/nit "attempt to find a transform"

src/cfg.cpp Outdated Show resolved Hide resolved
@alex-courtis
Copy link
Owner Author

There is a large refactor incoming, which greatly affects cfg.

I am happy to merge it to your repo.

I'll need to have push access... applying a large merge as a patch is not viable.

@anandubey
Copy link

anandubey commented Jul 13, 2022

I have added you to the collaborators list. I hope now you will have the push access.

@alex-courtis
Copy link
Owner Author

I have added you to the collaborators list. I hope now you will have the push access.

Thanks @anandubey

I'll merge master to your fork after https://github.com/alex-courtis/way-displays/tree/49-yaml-schemas is merged.

@alex-courtis
Copy link
Owner Author

I'll merge master to your fork after https://github.com/alex-courtis/way-displays/tree/49-yaml-schemas is merged.

master at 1.6.0 has been merged, along with the patches above.

The large change resulted from refactors when adding the IPC API. A lot of src/cfg was moved to src/marshalling

I've pre-emptively added !!transform to the YAML schema
You'll need test it with the example client when you have finished adding all the TRANSFORM enums.

@alex-courtis
Copy link
Owner Author

Nudge @anandubey

Any updates on this one?

This is really close to complete and I look forward to releasing it!

@anandubey
Copy link

@alex-courtis Sorry for zoning out on this. Started my new job on 1st Aug. It's a rust dev role, so all my weekends are going in that. I will try my best to finish it this week.

@alex-courtis
Copy link
Owner Author

@alex-courtis Sorry for zoning out on this. Started my new job on 1st Aug. It's a rust dev role, so all my weekends are going in that. I will try my best to finish it this week.

No worries. There is no hurry at all.

Good luck in the new role!

@edrex
Copy link
Collaborator

edrex commented Feb 28, 2023

Thanks for your work on this @anandubey, and totally understand about prioritizing paid job.

It would be great to get output transforms in tho. Do you anticipate having time to get this merge ready going forward, or alternatively, would you be open to handing it off?

If C was fresher in my mind I would volunteer (and my ADHD brain likes to say yes) but I shouldn't.

@alex-courtis
Copy link
Owner Author

alex-courtis commented Mar 4, 2023

would you be open to handing it off?

That would be fantastic!

I just cannot allocate the time to finish this one off...

If C was fresher in my mind I would volunteer (and my ADHD brain likes to say yes) but I shouldn't.

make cppcheck might be useful to you.

way-displays is also set up for ccls live checking in the editor. Here's my config for neovim lsp: https://github.com/alex-courtis/arch/blob/bd011f38a006cecc2f32526b944d145e164dd7d2/config/nvim/lua/amc/plugins/lsp.lua#L11

@edrex
Copy link
Collaborator

edrex commented Mar 4, 2023

make cppcheck

cool!

way-displays is also set up for ccls live checking in the editor.

alright! I use helix which uses clangd by default but I can change it in languages.toml.

@alex-courtis
Copy link
Owner Author

alex-courtis commented Mar 5, 2023

alright! I use helix which uses clangd by default but I can change it in languages.toml.

All good; clang is preferred and used by ccls. gcc is only used by default for legacy packaging reasons.

@alex-courtis
Copy link
Owner Author

Added development information to https://github.com/alex-courtis/way-displays/blob/master/CONTRIBUTING.md

@edrex
Copy link
Collaborator

edrex commented Apr 10, 2023

Want to try to get this in before #83? I would be willing to do a testing pass and make any needed changes if you have time for code review. I think I basically remember how to C :)

I'll be on a road trip/camping Apr 14-23 though, so we'll see how much keyboard time I get.

@alex-courtis
Copy link
Owner Author

alex-courtis commented Apr 10, 2023

Want to try to get this in before #83? I would be willing to do a testing pass and make any needed changes if you have time for code review. I think I basically remember how to C :)

I'll be on a road trip/camping Apr 14-23 though, so we'll see how much keyboard time I get.

Oh yes please!

I've added a lot of unit tests on master, so that should make this easier RE merge etc.

Edit: Rather than trying to merge this branch it might be easier just to create a new branch and manually copy the relevant bits. Things have changed a lot since this was raised.

@alex-courtis alex-courtis mentioned this pull request Apr 10, 2023
9 tasks
@alex-courtis alex-courtis mentioned this pull request May 6, 2023
6 tasks
@alex-courtis alex-courtis modified the milestones: 1.9.0 Roadmap, 1.9 Jun 24, 2023
@alex-courtis
Copy link
Owner Author

#133

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.

Feature: Transformation
3 participants