API feature #235

JezerM · 2021-08-26T03:25:19Z

Closes #234

Description

Adds the ability to update i3lock through a piping API.
Enable this with --api-enable flag
~~It is insecure. Send --no-verify and screen is unlocked.~~ (Fixed)

Screenshots/screencaps

Video.25-8-21.21.01.05.mov

Release notes

Notes: Added API feature

Rio6

Do we really need to include word expansion into i3lock-color? I feel that should be the job for the scripts writing the the fifo and not here.

Rio6 · 2021-08-26T21:37:59Z

i3lock.c

+pthread_t api_thread;
+// this enables api access to update i3lock
+bool api_enabled = false;
+char *api_fifo_path = "/tmp/i3lock-api";


We can probably utilize XDG_RUNTIME_DIR environment variable before falling back to a default (in addition to specifying a path from command line).

i3lock.c

Rio6 · 2021-08-26T21:53:23Z

i3lock.c

@@ -1437,19 +1489,10 @@ static void load_slideshow_images(const char *path, char *image_raw_format) {
    closedir(d);
 }

-int main(int argc, char *argv[]) {


This might make merging from upstream harder, although I don't have a better way for now

@Raymo111 I think you're usually the one merging from upstream, what do you think?

Yeah this would probably pose issues down the line, can we make this work without shrinking main by that much?

I wonder if it's possible to rearrange the code a bit to reduce the diff size. Seems like most of the diff are in this file?

JezerM · 2021-08-30T05:49:28Z

Changes

Using custom parser, instead of wordexp
Arguments are now limited
No big changes to main
There's thread locking on redraw_screen to avoid unnecessary and problematic redraws at the same time.
Images working! (kinda)

Considerations

Image updates work, but not as expected. Could lead to performance issues and a high chance of crashing. Better to use i3lock-color slideshow feature.
Multiple updates at short intervals might not be seen: set --refresh-rate to a lower value.

Video.29-8-21.23.29.59.mov

api.c

api.h

api.c

i3lock.c

api.c

JezerM · 2021-09-14T22:32:33Z

Changes

parse_args kind of refactored.
getopt_long moved from main to update_arguments, which can be accessed by listen_api
--enable-api flag renamed to --api-enable
- Added --api-path flag to change the api fifo path
- Added --api-redraw flag to force redraw when an api message is received
Added support for this kind of program
Changing image with --image flag through api does not crash as before.

JezerM · 2022-01-10T18:38:58Z

Setting images through API works without any issue (no crashes):

Upload.from.GitHub.for.iOS.MOV

However, big files at low intervals can slow i3lock if your graphics are slow:

Upload.from.GitHub.for.iOS.MOV

Rio6

Besides the comments in code, I think it looks good.

~~I'm personally not a big fan of parse_args related code (maybe something like wordexp(3) can be used instead). But it's probably fine for now.~~

api.c

Rio6 · 2022-01-12T02:07:39Z

api.c

+                text[i] = c;
+            i++;
+        }
+        text[i] = '\0';


Is the size of text is always larger than i here?

Yep, as we increase the memory of text by 2 if the size of text is lower or equal than i. It should always be enough memory to insert the NULL character.

What about the case when in the beginning of the loop i == size - 1. The loop doesn't get extended and i++ gets executed. If the loop exits at this iteration, i == size and writing to text[size] is out of bounds.

Sure, it would be out of bounds. if (size <= i + 1) { /* reallocate */ } should do the thing.

api.c

i3lock.c

i3lock.h

Rio6 · 2022-01-12T02:19:35Z

i3lock.c

@@ -1437,19 +1489,10 @@ static void load_slideshow_images(const char *path, char *image_raw_format) {
    closedir(d);
 }

-int main(int argc, char *argv[]) {


I wonder if it's possible to rearrange the code a bit to reduce the diff size. Seems like most of the diff are in this file?

JezerM · 2022-01-12T03:16:15Z

But wordexp performs word expansion, something that i3lock shouldn't do, as you stated before.

I don't think we need wordexp here though, just need a simple parser to parse escaped space and quotes.

Rio6 · 2022-01-12T04:21:11Z

But wordexp performs word expansion, something that i3lock shouldn't do, as you stated before.

I don't think we need wordexp here though, just need a simple parser to parse escaped space and quotes.

Sorry I'm incinsistent, ignore me on this whole parsing part. I'm not sure which way I like either :P.

I looked at my old comment and I agree with my previous self, wordexp does too much. I completely forgot what it does other than the function name. It's good right now.

Naheel-Azawy · 2022-02-18T11:25:39Z

Hi, this looks pretty good. Why it hasn't been merged yet?

JezerM · 2022-02-18T15:47:09Z

Hi, this looks pretty good. Why it hasn't been merged yet?

This feature is huge, and it does some big changes inside the main function, which could make it difficult to pull from i3lock.

Besides, even if this feature looks cool, it's mostly unnecessary. It's an extra that does a lot~

Raymo111 · 2022-02-18T18:35:51Z

Yeah what @JezerM said basically.

Jez I think I'll close this PR for now, and if we really need it someday we can reconsider it. Thanks for putting in all the effort still, it means a lot. If you'd like you can maintain an API branch/fork and keep it up to date with master so that if someone wants the feature they can use your fork?

JezerM · 2022-02-18T18:47:03Z

Yeah, I could~

Rio6 reviewed Aug 26, 2021

View reviewed changes

Added api.c, custom args parser, and limited to certain arguments.

7995b90

JezerM force-pushed the api-fifo branch from e55ea82 to 7995b90 Compare August 30, 2021 05:24

Rio6 reviewed Aug 30, 2021

View reviewed changes

Moved getopt_long from main to update_arguments. parse_args refactored

9f6e69f

JezerM mentioned this pull request Sep 16, 2021

Slideshow Image Loading Improvements #241

Closed

2 tasks

JezerM and others added 3 commits September 19, 2021 12:55

API options are now whitelisted from i3lock options

af1b3a9

Merge branch 'master' into api-fifo

9960e9c

Merge branch 'master' into api-fifo

df1453f

JezerM marked this pull request as ready for review September 19, 2021 19:15

Revert indent

902d896

JezerM and others added 2 commits January 10, 2022 13:54

Merge branch master into api-fifo

f9836a7

Merge branch 'master' into api-fifo

d4b33ac

Rio6 reviewed Jan 12, 2022

View reviewed changes

Fix memory allocation and some formatting.

a9d788a

Raymo111 closed this Feb 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API feature #235

API feature #235

JezerM commented Aug 26, 2021 •

edited

Loading

Rio6 left a comment

Rio6 Aug 26, 2021

Rio6 Aug 26, 2021

Rio6 Aug 27, 2021

Raymo111 Aug 27, 2021

Rio6 Jan 12, 2022

JezerM commented Aug 30, 2021

JezerM commented Sep 14, 2021

JezerM commented Jan 10, 2022

Rio6 left a comment •

edited

Loading

Rio6 Jan 12, 2022

JezerM Jan 12, 2022 •

edited

Loading

Rio6 Jan 12, 2022

JezerM Jan 12, 2022

Rio6 Jan 12, 2022

JezerM commented Jan 12, 2022

Rio6 commented Jan 12, 2022 •

edited

Loading

Naheel-Azawy commented Feb 18, 2022

JezerM commented Feb 18, 2022 •

edited

Loading

Raymo111 commented Feb 18, 2022

JezerM commented Feb 18, 2022

API feature #235

API feature #235

Conversation

JezerM commented Aug 26, 2021 • edited Loading

Description

Screenshots/screencaps

Release notes

Rio6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JezerM commented Aug 30, 2021

Changes

Considerations

JezerM commented Sep 14, 2021

Changes

JezerM commented Jan 10, 2022

Rio6 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JezerM Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JezerM commented Jan 12, 2022

Rio6 commented Jan 12, 2022 • edited Loading

Naheel-Azawy commented Feb 18, 2022

JezerM commented Feb 18, 2022 • edited Loading

Raymo111 commented Feb 18, 2022

JezerM commented Feb 18, 2022

JezerM commented Aug 26, 2021 •

edited

Loading

Rio6 left a comment •

edited

Loading

JezerM Jan 12, 2022 •

edited

Loading

Rio6 commented Jan 12, 2022 •

edited

Loading

JezerM commented Feb 18, 2022 •

edited

Loading