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

Signal rename #623

Merged
merged 2 commits into from Jul 26, 2020
Merged

Signal rename #623

merged 2 commits into from Jul 26, 2020

Conversation

ammen99
Copy link
Member

@ammen99 ammen99 commented Jul 24, 2020

Fixes #108

I am sorry for all of you who have custom plugins (@damianatorrpm @myfreeweb @soreau), this will very likely break compilation. At least with this PR all signal names should be consistent, all signal structs also follow the same naming convention and are all in the wf:: namespace. Also, I have added documentation for all signals emitted from core.

Here is an incomplete porting guide:

  1. Most events are the same, the type has changed view_geometry_changed_signal -> wf::view_geometry_changed_signal
  2. unmap-view, pre-unmap-view, map-view are renamed to view-unmapped, view-pre-unmapped and view-mapped
  3. attach-view, detach-view, layer-attach-view, layer-detach-view are now view-(layer-)attached and view-(layer-)detached.
  4. view-disappeared now is emitted together with all the view-detached, view-unmapped and view-minimized signals.
  5. focus-view -> view-focused
  6. view-maximize-request -> view-tile-request

@unrelentingtech
Copy link
Contributor

unrelentingtech commented Jul 24, 2020

Is that consistent enough? Looks like not enough *ed to me :D Why not view-pre-unmapped, view-tile-requested?

@ammen99
Copy link
Member Author

ammen99 commented Jul 24, 2020

Why not view-pre-unmapped, view-tile-requested

The -request part is a noun, but will change the other one too :)

@ammen99
Copy link
Member Author

ammen99 commented Jul 24, 2020

Also, my main concern was the order. In master, we have: map-view, move-request, view-maximize-request ... I myself need to look up the order each time :)

@damianatorrpm
Copy link
Contributor

damianatorrpm commented Jul 25, 2020

for the sake of consistency maybe you should rename view-move-to-output to also have a -pre- and emit it without the -pre- at the end of the function with the same data structure when it actually moved

@@ -726,6 +726,8 @@
     new_output->workspace->add_view(v,
         v->minimized ? wf::LAYER_MINIMIZED : wf::LAYER_WORKSPACE);
     new_output->focus_view(v);
+
+    this->emit_signal("view-moved-to-output", &data);  

@kode54
Copy link
Contributor

kode54 commented Jul 25, 2020

You forgot to apply the settings to src/view/xwayland.cpp, which also implies that CI is not verifying whether the Xwayland bits build.

@ammen99
Copy link
Member Author

ammen99 commented Jul 25, 2020

@kode54, what do you mean? CI builds the whole project and uncrustify runs on all cpp and hpp files.

@kode54
Copy link
Contributor

kode54 commented Jul 25, 2020

Oh, hmm, maybe the errors were due to another merge request in my test queue. Oh well, I'll leave whoever authored that merge to fix their thing when this lands.

@ammen99 ammen99 merged commit 8e2c1c1 into master Jul 26, 2020
6 checks passed
@ammen99 ammen99 deleted the signal-rename branch Jul 26, 2020
@ammen99
Copy link
Member Author

ammen99 commented Jul 26, 2020

For people who are trying to fix their plugins, here are the commands I have used for the main plugins: https://pastebin.com/44igfdvH. The commands do some things wrongly at first but if you run all of them you should get correct results. Anyway, do check what changed.

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.

Ensure consistency in signal naming
4 participants