Allow packages to register scripts declaratively

[LiberalArtist]

This is incomplete and untested (I haven't even looked at updating the test suite yet), but here is a draft of my attempt at resolving https://github.com/Metaxal/quickscript/issues/73. I'll comment with some more thoughts tomorrow.

[rfindler]

Matthew recently had some issues with this actually (personal email from Dec 15, 2023, cc robby): when working on the GUI, launching DrRacket was getting slow because some script compilation triggered compilation of the GUI @rfindler said he'd think about it. Discussion still ongoing.

My thoughts didn't turn up much, unfortunately. The suggestion Matthew had is that quickscript should use the manager-skip-file-handler as drracket already does, which seems like a good idea to me.

In that thread, @Metaxal suggested to stop auto-compiling scripts, unless the scripts are outside the package system (this would probably also require the change in the previous paragraph). That seems like a good idea to me, especially if we're moving to a situation where, by default, there are no scripts outside the package system.

[Metaxal]

Another simple idea was to actually delete the .zo files for user-based scripts, and remove the compilation part altogether.

[rfindler]

Another simple idea was to actually delete the .zo files for user-based scripts, and remove the compilation part altogether.

Yes, that might be a good idea. Then the script files become like other files in racket and people can expect to manage the compilation of them on their own.

[LiberalArtist]

Re https://github.com/Metaxal/quickscript/issues/73#issuecomment-1880755277:

• I'm okay with using the preference system. I'm just worried that the prefs file is getting big and any change of any pref requires rewriting the whole file, which becomes slower and slower as we add more prefs. Or can we use a separate quickscripts-prefs.rktd file maybe?

Looking at my own racket-prefs.rktd, existing prefs like readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos seem to store much more data than I'd expect for my proposed plt:quickscript:library, which only holds directories and exclusions explicitly added by the user. I'd defer to @rfindler on the proper use of the preferences system, though.

It does seem possible to use a separate quickscripts-prefs.rktd file, and it would still let us rely on support from the preferences system for locking, compatibility across versions, etc., but it seemed better to avoid the complexity of a separate file unless it is actually beneficial. `

* I'm okay with keeping things consistent with `test-include-paths` and friends, except that `all` doesn't seem adequate for quickscripts (`info.rkt` is not a script for example), and it seems to me that the default should be a `scripts` subdirectory instead — or automatically skipping files like `info.rkt` but that seems fragile.

I could also imagine (define quickscript-directory #t) and document that values other than #t are reserved for future use.

Then maybe just (define quickscript-directory "scripts") instead?

I ended up going with quickscript-directory instead of imitating test-include-paths et al. for two main reasons:

1. The existing UI and other parts of the design have a model that script directories contain script files (as immediate children, with constrained file names), which may be included or included. Keeping that model helped to minimize the disruption from these changes. In particular, that meant I wanted the info.rkt entries to only specify directories. That's different from entries like test-include-paths, which can specify individual files and, when they specify a directory, apply to the directoy's recursive contents instead of only its immediate contents.
2. I'd hoped there would be a library function to handle matching test-include-paths-style specifications, but there doesn't seem to be. (Some of the examples, like compile-omit-paths, have to work under extra restrictions.) The flexibility those specifications allow seems to come with some pitfalls, like the potential for "../../../../etc/passwd", which I'm not sure even the current uses handle.

I can see how (define quickscripts-directory "scripts") would allow a package like quickscript-extra to avoid having to add an additional info.rkt file, and I think it would be viable to allow a value of a single collection-name-element?.

I like that the current (define quickscript-directory #t) would allow you to create a package of only QS scripts by adding just an info.rkt file, without having to move the scripts to a subdirectory. (Github Gists do not allow subdirectories, for example.) A side benefit is that, if we accept the restriction that (script-file? "info.rkt") is #false, putting the info.rkt file in the script directory prevents accidentally attempting to create a script with that name.

One issue though: quickscript-test comes with a bunch a scripts that should loaded only when running the test suite. So these scripts must be excluded by default. How could this happen? (Currently, these scripts are not included by default, and registered only for the duration of the tests.)

I hadn't realized https://github.com/Metaxal/quickscript-test existed! I haven't looked at it yet, but I expect it should not use the info.rkt mechanism and instead only add its script directory when setting up to run tests (and remove it on teardown, or else not save it).

It want to look into how other things based on find-relevant-directories handle testing.

The new mechanism also requires the creation of a collection to be able to add a directory with scripts. It's heavier than the un/register approach and can't be done (easily) directly within the library GUI I suspect.

I suppose we could have both: (i) explicit exclusions of find-relevant-directories and (ii) explicit inclusions of user-added paths.

(i) would be for collections and (ii) would be for non-collections.

Thoughts?

I think this PR already does what you propose. It is still possible to use the GUI to add a new non-collection directory just as before.

* Do keep in mind how to create shadow scripts from existing collection-based scripts. (I don't think I have a test for this though, since it's a little tricky as it depends on adding ad-hoc collections and stuff. Not a good excuse anyhow.)

* The library should still show all the scripts that are loaded or excluded.

There is a bug with shadowing collection-based scripts currently, but these are both supposed to work.

[rfindler]

• I'm okay with using the preference system. I'm just worried that the prefs file is getting big and any change of any pref requires rewriting the whole file, which becomes slower and slower as we add more prefs. Or can we use a separate quickscripts-prefs.rktd file maybe?

Looking at my own racket-prefs.rktd, existing prefs like readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos seem to store much more data than I'd expect for my proposed plt:quickscript:library, which only holds directories and exclusions explicitly added by the user. I'd defer to @rfindler on the proper use of the preferences system, though.

We have had some performance troubles in the past with preferences getting too large in certain situations and there's even some support built into DrRacket so we can get information from users to debug this kind of problem. Wave your mouse just to the left of the memory in the bottom right of the DrRacket window, where this arrow is pointing:

and you should see an ugly "P" appear. Click on it to get some information about the preferences.

I don't think we're in the "use a different mechanism" territory here. I'd say that the current situation is probably fine and, absent performance problems, saving the preferences to the disk on write is preferable. But there could be some improvements and they'd be welcome if someone has the energy.

[Metaxal]

@LiberalArtist All fine with me.

Regarding the single preference file, personally I find putting everything in there somewhat jarring, efficiency-wise and conceptually — but I can certainly understand historical reasons and organic development. In particular the interaction history should be in its own separate ad-hoc pref file, as well as recently-opened files, and any other sub-preferences that can take an arbitrary amount of space. Like the QS library.

Splitting in files also allows for the user to throw away only one file when something goes wrong (say, getting rid of the history, or throwing away the QS preferences), without having to carefully edit a large data file with everything it. I'm not asking to change the existing preferences here, but I think if we can keep a separate preferences file for QS, that would be better.

[LiberalArtist]

Re @Metaxal:

Regarding the single preference file, personally I find putting everything in there somewhat jarring, efficiency-wise and conceptually — but I can certainly understand historical reasons and organic development. In particular the interaction history should be in its own separate ad-hoc pref file, as well as recently-opened files, and any other sub-preferences that can take an arbitrary amount of space. Like the QS library.

Splitting in files also allows for the user to throw away only one file when something goes wrong (say, getting rid of the history, or throwing away the QS preferences), without having to carefully edit a large data file with everything it. I'm not asking to change the existing preferences here, but I think if we can keep a separate preferences file for QS, that would be better.

In https://github.com/LiberalArtist/quickscript/commit/87b4630c78ba536565f1761c15f83354cc173a20 I've tried using a separate quickscript-prefs.rktd file. I don't like it, though.

With the benefit of hindsight, I would also prefer for readline-input-history, plt:framework-pref:drracket:console-previous-exprs, plt:framework-pref:drracket:recently-closed-tabs, and plt:framework-pref:framework:recently-opened-files/pos not to be stored in (find-system-path 'pref-file) or even (find-system-path 'pref-dir): I like the organization of the XDG Base Directory Specification, which puts such things in XDG_STATE_HOME, as opposed to XDG_CONFIG_HOME or XDG_DATA_HOME. (Even there, this distinction was only added in 2021!)

The Quickscript library, though, seems to me like it really is a user configuration preference. It is also much smaller than the large "preferences" I listed, in part because it stores only exclusions and non-collection directories. (Handling user-script-dir specially per https://github.com/Metaxal/quickscript/pull/81#discussion_r1458189845 would let it be even smaller.) Concretely, my racket-prefs.rktd file is 31,688 bytes, while, with two shadow scripts, my stand-alone quickscript-prefs.rktd file is the following 528 bytes:

(
 (plt:framework-pref:plt:quickscript:library
  (
   (3)
   2
   ((#"/home/philip/code/racket/pkgs/quickscript/private/library.rkt" . deserialize-info:library-data-v0) ((lib "racket/private/set-types.rkt") . deserialize-info:immutable-custom-set-v0))
   0
   ()
   ()
   (0 (h - (equal-always) ((p+ #"/home/philip/.config/racket/quickscript/user-scripts/" . unix) 1 #f (h - (equal-always)))) (1 #f (h - (equal-always) ((q lib "quickscript/scripts/eyes.rkt") . #t) ((q lib "quickscript-extra/scripts/tweet.rkt") . #t))))
  ))
)

While parameterizing preferences:low-level-get-preference and preferences:low-level-put-preferences works in https://github.com/LiberalArtist/quickscript/commit/87b4630c78ba536565f1761c15f83354cc173a20, it seems like it would complicate making deeper use of the preference system in the future (including the debugging info @rfindler mentioned in https://github.com/Metaxal/quickscript/pull/81#issuecomment-1882008361). More immediately, it seems like it may require adding more custom hooks for testing, since any attempt to override those parameters for testing would now be overridden by our parameterization.

All of that said, I think even using a separate quickscript-prefs.rktd seems better than managing all the file IO, caching, and failure issues manually.

[Metaxal]

You have a point that exclusion should be more sparse than inclusion — except if someone makes extensive use of shadow scripts, which is not unlikely.

I agree that using the preference system is better than a custom file. IIUC however, it seems that the preference system is not currently well suited to have multiple preference files, is that what you're saying?

Assume we choose to use the common racket-prefs.rktd file for QS. How much hassle would it be to later switch to a custom quickscript-prefs.rktd versus switching now? If it's at least the same effort, then let's use racket-prefs.rktd for now. Otherwise let's assess. (@rfindler may have a more informed opinion on this matter.)

It still feels pretty wrong to me that a plugin would use the common racket-prefs.rktd :stuck_out_tongue:

[rfindler]

If it's at least the same effort, then let's use racket-prefs.rktd for now. Otherwise let's assess. (@rfindler may have a more informed opinion on this matter.)

I think using the framework preferences system is a good choice. It does a lot of things for you that are easy to get wrong. I wouldn't worry about "racket-lang.org" appearing in the filename. Check Syntax is a plugin, after all.

That said, I do think that @LiberalArtist makes a good point that it might be beneficial to move more "state"-like preferences into their own place. It seems better for the usecase of people actually editing the preferences file. There may be other reasons to want to do it too.

[Metaxal]

How is the progress going @LiberalArtist ? Any blocker?

[LiberalArtist]

How is the progress going @LiberalArtist ? Any blocker?

I'm expecting to have time to get back to this in the next few weeks. Having missed 8.13, I'd like to get this done soon so we have some real-world experience before 8.14. I'll have to page the context back in, but IIRC it was fairly close: I think I was working on the test suite.

[Metaxal]

Thanks for the update!

On Sat, Apr 27, 2024 at 2:18 AM Philip McGrath @.***> wrote:

How is the progress going @LiberalArtist https://github.com/LiberalArtist ? Any blocker?

I'm expecting to have time to get back to this in the next few weeks. Having missed 8.13, I'd like to get this done soon so we have some real-world experience before 8.14. I'll have to page the context back in, but IIRC it was fairly close: I think I was working on the test suite.

— Reply to this email directly, view it on GitHub https://github.com/Metaxal/quickscript/pull/81#issuecomment-2080298059, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMU2HHK7WWRUVLTE4CFZH3Y7L4EZAVCNFSM6AAAAABBQECFEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBQGI4TQMBVHE . You are receiving this because you were mentioned.Message ID: @.***>