rabbibotton / clog

CLOG - The Common Lisp Omnificent GUI
Other
1.48k stars 101 forks source link

Boot file config #237

Closed mmontone closed 1 year ago

mmontone commented 1 year ago

Hi.

I'm struggling a bit with the setting up of a custom boot file. Part of the problem is that CLOG doesn't fail if my boot file path is wrong (it is wrong because CLOG does some path transformations).

I think it should fail if boot-file path is set AND it is not possible to open the file.

I would change :if-does-not-exist to :error.

https://github.com/rabbibotton/clog/blob/abd6798912c51e18168a8417550fc946b83368e6/source/clog-connection.lisp#L446

rabbibotton commented 1 year ago

Line 450 the if should instead of returning "" should signal an error based on what you are asking. Can you test that on your end (sorry power in an out right now post hurricane).

(let ((page-data (if stream
                                               (make-string (file-length stream))
                                               (if static-boot-html
                                                   ""
                                                   (compiled-boot-html nil nil))))
mmontone commented 1 year ago

Oh. Hope things are fine there after the hurricane. :eyes:

I see your remark, but I don't understand. My point is this: if boot-file is set, then it makes sense to signal an error if it is not found, and not "fail silently" (the code falling back to something else). If no boot-file is desired, then it should be set to nil to be ignored explicitly.

If you set the boot file to something invalid or inexistent, the code does not fail because of the :on-error is nil. And I think it'd be better if it failed.

I'm not expert on the internals, just an issue I had and solved already. It is my preference that things fail when not setup correctly, instead of the code going through other path. (Javascript comes to my mind).

mmontone commented 1 year ago

This is not big issue, btw. I'll leave it to you if you want to do something about it or not.

mmontone commented 1 year ago

Line 450 the if should instead of returning "" should signal an error based on what you are asking. Can you test that on your end (sorry power in an out right now post hurricane).

(let ((page-data (if stream
                                               (make-string (file-length stream))
                                               (if static-boot-html
                                                   ""
                                                   (compiled-boot-html nil nil))))

Ah. I see. I'll test... But why not change nil by :error in :if-does-not-exist ?

mmontone commented 1 year ago

When I set an error there, it is never triggered.

If I give an invalid boot-file, clog does not complain and "works" with some default boot file.

If I set :if-does-not-exist to :error then it works as I would expect: signal error when boot file path is invalid.

rabbibotton commented 1 year ago

I assume you set static-boot-html to t the default, nil, uses a pre-compiled boot.html if the boot file does not exist.

current: If static-boot-html is t then if the file fails to load it currently becomes "" I am going to test setting stat-boot-html to a string (user defined html error) if boot file not found or can not be loaded and if set to :error will signal an error.

rabbibotton commented 1 year ago

Ok I have pushed it. Let me know if that works for you.

So for example:

(clog:initialize nil :static-boot-html :error)
(clog:set-on-new-window (lambda (body) (clog:create-div body :content "test")) :boot-file "/x.html" :path "/test")

would signal an error

(clog:initialize nil :static-boot-html "Boot file not found")
(clog:set-on-new-window (lambda (body) (clog:create-div body :content "test")) :boot-file "/x.html" :path "/test")

Would return a page that states - Boot file not found

mmontone commented 1 year ago

Thanks. That's a better.

I would have preferred to signal error when I set bad :boot-file by default, like here:

(clog:initialize 'start-desktop :boot-file "invalid-file")

But you must have reasons.

Cheers,