Use xcb-errors library if it is available #2665

psychon · 2019-02-17T14:14:01Z

This library allows to get a human-readable string describing X11
requests, events, and errors. We now use this library to pretty-print
X11 errors if we get any.

To test this code, I added the following two lines to AwesomeWM so that
X11 errors are generated:

xcb_set_input_focus(globalconf.connection, 42, 42, 42);
xcb_randr_set_output_primary(globalconf.connection,
    globalconf.screen->root, 42);

Output without xcb-errors:

X error: request=SetInputFocus (major 42, minor 0), error=BadValue (2)
X error: request=(null) (major 140, minor 30), error=(null) (147)

Output with xcb-errors:

X error: request=SetInputFocus (major 42, minor 0), error=Value (2)
X error: request=RandR-SetOutputPrimary (major 140, minor 30), error=RandR-BadOutput (147)

Signed-off-by: Uli Schlachter [email protected]

One still open question

Should (one of the) travis build(s) install xcb-errors to test compilation of this?

codecov · 2019-02-17T14:17:58Z

Codecov Report

Merging #2665 into master will decrease coverage by <.01%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##           master    #2665      +/-   ##
==========================================
- Coverage   86.23%   86.22%   -0.01%     
==========================================
  Files         529      529              
  Lines       36401    36406       +5     
==========================================
+ Hits        31390    31391       +1     
- Misses       5011     5015       +4

Flag	Coverage Δ
#gcov	`75.61% <16.66%> (-0.04%)`	⬇️
#luacov	`89.03% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
awesome.c	`78.5% <ø> (ø)`	⬆️
globalconf.h	`100% <ø> (ø)`	⬆️
event.c	`72.62% <0%> (-0.64%)`	⬇️
common/version.c	`84.21% <100%> (+0.87%)`	⬆️

actionless · 2019-02-17T23:50:47Z

* Should (one of the) travis build(s) install xcb-errors to test compilation of this?

i think yes

blueyed · 2019-02-18T01:29:31Z

awesomeConfig.cmake

+        set(WITH_XCB_ERRORS OFF)
+        message(STATUS "xcb-errors not found. Disabled.")
+    endif()
+endif()


Could this default to AUTO, and cause an error if ON was used, but it is not available?

There is no AUTO for option(). It is a TRUE / FALSE thing. I briefly looked into it (it is possible, just not with option()) and then decided to just copy what we do for DBus, since apparently that was good enough so far.

If you disagree, I can look into implementing AUTO for WITH_DBUS, GENERATE_MANPAGES, GENERATE_DOC, and anything else that needs this. But I'd do that in a separate PR.

Sure, just a quick idea.
I think it would be nice in general, but not really necessary.

.travis.yml

Previously, these options had two possible values: ON and OFF. Now, they have three possible values: ON, OFF, and AUTO. OFF still does what it always did: The feature is just disabled. With ON and AUTO, we check for the feature. The difference is what happens when the feature could not be enabled, e.g. because some dependencies is missing: With AUTO, we just disable the feature (this is what happened previously with ON). However, with ON, CMake aborts and prints an error. Implements: Suggestion by Daniel awesomeWM#2665 (review) Signed-off-by: Uli Schlachter <[email protected]>

blueyed · 2019-02-18T21:35:58Z

Let's have this (rebased) after #2671.

Previously, these options had two possible values: ON and OFF. Now, they have three possible values: ON, OFF, and AUTO. OFF still does what it always did: The feature is just disabled. With ON and AUTO, we check for the feature. The difference is what happens when the feature could not be enabled, e.g. because some dependencies is missing: With AUTO, we just disable the feature (this is what happened previously with ON). However, with ON, CMake aborts and prints an error. Implements: Suggestion by Daniel awesomeWM#2665 (review) Signed-off-by: Uli Schlachter <[email protected]>

blueyed · 2019-02-19T19:42:48Z

Should also come after #2682 (for addons), but not really necessary.

actionless · 2019-02-20T14:06:49Z

@psychon so #2682 got merged

psychon · 2019-02-20T14:23:11Z

Rebased and updated for those two other PRs.

common/version.c

blueyed · 2019-02-21T03:42:10Z

.travis.yml

@@ -65,13 +65,15 @@ jobs:
            - liblua5.3-dev
            - lua5.3
    # Note: luarocks does not work with Lua 5.0.
-    - env: LUA=5.1 LUANAME=lua5.1 BUILD_IN_DIR=/tmp/awesome-build
+    - env: LUA=5.1 LUANAME=lua5.1 BUILD_IN_DIR=/tmp/awesome-build WITH_XCB_ERRORS=yes


Sorry for not noticing earlier, but it would be better to add this to the DO_COVERAGE=codecov job.

Went ahead and amended your last commit.

actionless · 2019-02-22T16:47:07Z

@blueyed it have a conflict again

actionless · 2019-03-03T08:06:59Z

@psychon and again :-)

This library allows to get a human-readable string describing X11 requests, events, and errors. We now use this library to pretty-print X11 errors if we get any. To test this code, I added the following two lines to AwesomeWM so that X11 errors are generated: xcb_set_input_focus(globalconf.connection, 42, 42, 42); xcb_randr_set_output_primary(globalconf.connection, globalconf.screen->root, 42); Output without xcb-errors: X error: request=SetInputFocus (major 42, minor 0), error=BadValue (2) X error: request=(null) (major 140, minor 30), error=(null) (147) Output with xcb-errors: X error: request=SetInputFocus (major 42, minor 0), error=Value (2) X error: request=RandR-SetOutputPrimary (major 140, minor 30), error=RandR-BadOutput (147) Signed-off-by: Uli Schlachter <[email protected]>

Signed-off-by: Uli Schlachter <[email protected]>

psychon · 2019-03-03T08:17:23Z

Rebased on master

Previously, these options had two possible values: ON and OFF. Now, they have three possible values: ON, OFF, and AUTO. OFF still does what it always did: The feature is just disabled. With ON and AUTO, we check for the feature. The difference is what happens when the feature could not be enabled, e.g. because some dependencies is missing: With AUTO, we just disable the feature (this is what happened previously with ON). However, with ON, CMake aborts and prints an error. Implements: Suggestion by Daniel awesomeWM#2665 (review) Signed-off-by: Uli Schlachter <[email protected]>

blueyed reviewed Feb 18, 2019

View reviewed changes

.travis.yml Show resolved Hide resolved

psychon force-pushed the util-errors branch from b277d87 to 2b5762c Compare February 18, 2019 07:53

psychon mentioned this pull request Feb 18, 2019

CMake: Add "AUTO" state for WITH_DBUS and GENERATE_MANPAGES #2671

Merged

psychon force-pushed the util-errors branch from 2b5762c to 4039ea8 Compare February 18, 2019 20:25

psychon force-pushed the util-errors branch from 4039ea8 to b7009aa Compare February 20, 2019 14:22

actionless reviewed Feb 20, 2019

View reviewed changes

common/version.c Show resolved Hide resolved

actionless previously approved these changes Feb 20, 2019

View reviewed changes

blueyed reviewed Feb 21, 2019

View reviewed changes

blueyed dismissed actionless’s stale review via fecfce5 February 21, 2019 04:01

blueyed force-pushed the util-errors branch from b7009aa to fecfce5 Compare February 21, 2019 04:01

actionless previously approved these changes Feb 21, 2019

View reviewed changes

blueyed dismissed actionless’s stale review via c74068f February 21, 2019 04:02

blueyed force-pushed the util-errors branch from fecfce5 to c74068f Compare February 21, 2019 04:02

blueyed force-pushed the util-errors branch from c74068f to 59ca596 Compare February 23, 2019 09:20

blueyed previously approved these changes Feb 23, 2019

View reviewed changes

blueyed mentioned this pull request Feb 23, 2019

chore(common: version): replace unicode symbols to yes/no #2687

Merged

psychon added 2 commits March 3, 2019 09:15

.travis.yml: Install libxcb-errors in one of the builds

be0b565

Signed-off-by: Uli Schlachter <[email protected]>

psychon dismissed blueyed’s stale review via be0b565 March 3, 2019 08:17

psychon force-pushed the util-errors branch from 59ca596 to be0b565 Compare March 3, 2019 08:17

psychon requested review from actionless and blueyed March 3, 2019 08:17

actionless approved these changes Mar 3, 2019

View reviewed changes

blueyed approved these changes Mar 3, 2019

View reviewed changes

blueyed merged commit d21fd74 into awesomeWM:master Mar 3, 2019

psychon deleted the util-errors branch March 3, 2019 11:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use xcb-errors library if it is available #2665

Use xcb-errors library if it is available #2665

psychon commented Feb 17, 2019 •

edited

Loading

codecov bot commented Feb 17, 2019 •

edited

Loading

actionless commented Feb 17, 2019

blueyed Feb 18, 2019

psychon Feb 18, 2019

blueyed Feb 18, 2019

blueyed commented Feb 18, 2019

blueyed commented Feb 19, 2019 •

edited

Loading

actionless commented Feb 20, 2019

psychon commented Feb 20, 2019

blueyed Feb 21, 2019 •

edited

Loading

blueyed Feb 21, 2019

actionless commented Feb 22, 2019

actionless commented Mar 3, 2019

psychon commented Mar 3, 2019

Use xcb-errors library if it is available #2665

Use xcb-errors library if it is available #2665

Conversation

psychon commented Feb 17, 2019 • edited Loading

codecov bot commented Feb 17, 2019 • edited Loading

Codecov Report

actionless commented Feb 17, 2019

blueyed Feb 18, 2019

Choose a reason for hiding this comment

psychon Feb 18, 2019

Choose a reason for hiding this comment

blueyed Feb 18, 2019

Choose a reason for hiding this comment

blueyed commented Feb 18, 2019

blueyed commented Feb 19, 2019 • edited Loading

actionless commented Feb 20, 2019

psychon commented Feb 20, 2019

blueyed Feb 21, 2019 • edited Loading

Choose a reason for hiding this comment

blueyed Feb 21, 2019

Choose a reason for hiding this comment

actionless commented Feb 22, 2019

actionless commented Mar 3, 2019

psychon commented Mar 3, 2019

psychon commented Feb 17, 2019 •

edited

Loading

codecov bot commented Feb 17, 2019 •

edited

Loading

blueyed commented Feb 19, 2019 •

edited

Loading

blueyed Feb 21, 2019 •

edited

Loading