dark icon indicating copy to clipboard operation
dark copied to clipboard

Allow non-admins to view in-Editor debugger

Open StachuDotNet opened this issue 2 years ago • 8 comments

When working on Dark features, namely when adjusting how Fluid responds to my keyboard input, I keep reaching for the in-Editor Dark debugging experience currently only exposed to 'admins' (right now, that's just Paul). I've been locally getting around this restriction by editing Ui.fs, setting myself as an admin there.

This PR aims at exposing that functionality to anyone who marks themselves as a 'dark developer'. I'd like to use it in prod, both to diagnose my own Dark canvases, as well as potentially use the feature when diagnosing other users' issues.

Changes made:

  • A new "Editor" tab has been added to the settings modal, with a single radio button.
  • the editorSettings model has been changed - it no longer houses the runTimers, showHandlerASTs, and showFluidDebugger variables directly, but now rather houses these in a option<darkDev> field under editorSettings.
  • the sidebar debugger previously used a lot of vertical space in displaying the environment - now each key/value pair just takes up one line (unless it overflows) image
  • the SAVE STATE FOR INTEGRATION TEST feature no longer worked (since the OCaml->F# port?), so has been disabled (by commenting out relevant code).

StachuDotNet avatar Jul 28 '22 21:07 StachuDotNet

This is a draft, but some feedback would be appreciated, @pbiggar.

Does this overall direction seem right to you? Do those TODOs seem right to you?

I'll circle back to this after some input - no rush.

StachuDotNet avatar Jul 28 '22 21:07 StachuDotNet

If completed, this will help move #4276 forward.

StachuDotNet avatar Jul 28 '22 22:07 StachuDotNet

Ah - I just now saw this line in #4276:

It could be set in settings and stored along with the other options in LocalStorage.

I believe that implies I should move darkDev to editorSettings, and all of those settings would then be stored for me 'automatically' in LocalStorage. Great. That seems appropriate enough to me.

StachuDotNet avatar Jul 28 '22 22:07 StachuDotNet

Looks like a good idea!

pbiggar avatar Jul 29 '22 16:07 pbiggar

  • [x] This seems to break the rename_db_type integration test somehow

StachuDotNet avatar Aug 01 '22 16:08 StachuDotNet

  • [x] This seems to break the rename_db_type integration test somehow

This is likely caused by runTimer sneaking into darkDev. It's true by default in the original code - I thought this was a mistake, but seeing now that this was a mistake - it should live within editorSettings as it was.

I believe that the specific integration test failure (glad it was caught!) was that the analysis refresh timer wasn't running, so the DB (with data) didn't realize it had to "lock" itself, visually.

StachuDotNet avatar Aug 01 '22 18:08 StachuDotNet

Up-to-date with latest client changed, tidied, and ready for another review, @pbiggar (our last one was over Zoom)

StachuDotNet avatar Aug 08 '22 17:08 StachuDotNet

No worries! Yeah that works for me :+1:

StachuDotNet avatar Aug 10 '22 13:08 StachuDotNet

Handled by #4426

pbiggar avatar Aug 30 '22 16:08 pbiggar