Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Help/input wanted: Changes to dape config format and minibuffer read functionality #72

Open
svaante opened this issue Feb 14, 2024 Discussed in #71 · 1 comment
Open

Comments

@svaante
Copy link
Owner

svaante commented Feb 14, 2024

Discussed in #71

Originally posted by svaante February 14, 2024

Intro

I want some feedback on an dape issue that I have been think about for a while and something I want to get right, especially if the solution will break user defined configurations (calling it configs was an mistake, templates would have been better).

Let me just preface this to get your mind in the right place.

  1. The main goal of dape is to be as close to the emacs way of doing things.
  2. The dape configuration format is an plist not an json object.
  3. It's an plus if the user can infer the format of an dape configuration from an "launch.json" (necessary evil I think).

Problems

  1. Currently there is no way of using dape to remove or ignore an property in an configuration.
  2. Adapter capabilities needs to know each adapters Readme, wiki etc.

Improve usage ergonomics

If we use the following configuration as an example to illustrate the issue:

(gdb
 modes (c-mode c-ts-mode c++-mode c++-ts-mode)
 command-cwd dape-command-cwd
 command "gdb"
 command-args ("--interpreter=dap")
 fn (dape-config-tramp)
 :request "launch"
 :program "a.out"
 :cwd "."
 :env (:DEBUG 1)
 :stopAtBeginningOfMainSubprogram nil)

The user want to debug using the gdb config but does not want the environment variable DEBUG to be set.

There is two ways to work around this, both being painful:

  1. Add an new config to dape-configs where :env (:DEBUG 1) is removed
  2. Use dape command with launch :request "launch" :program "a.out" ... with add all properties from the gdb config without :env (:DEBUG 1)

Improve discoverability in batteries included configs

Let's reuse the same configuration.

(gdb
 modes (c-mode c-ts-mode c++-mode c++-ts-mode)
 command-cwd dape-command-cwd
 command "gdb"
 command-args ("--interpreter=dap")
 fn (dape-config-tramp)
 :request "launch"
 :program "a.out"
 :env (:DEBUG 1)
 :stopAtBeginningOfMainSubprogram nil)

If I where to add this configuration as an batteries included config it would look something like this.

(gdb
 modes (c-mode c-ts-mode c++-mode c++-ts-mode)
 command-cwd dape-command-cwd
 command "gdb"
 command-args ("--interpreter=dap")
 fn (dape-config-tramp)
 :request "launch"
 :program "a.out"
 :stopAtBeginningOfMainSubprogram nil)

Notice the missing :env property. Even though I know that :env exist I can't document the fact in the config without using some strange default value for :env to make it show up in dape completions and the hint section.

The user has to lookup the gdb manual to figure out that's the way to set environment variables.

Suggested solutions

Here follows some possible solutions. Split into the categories:

  • Change config format
  • Change dape command read functionality

Introduce an new special config value

One way would be to add an "null" or an "ignore" symbol.

  • In the first example: an user would then with dape command overwrite the config value with gdb :env some-new-symbol
  • In the second example: the property :env some-new-symbol can be added (the user still has to know that the value is an plist), which makes it show up in the hint buffer and as an completion.

JSON "null"

Add an symbol/keyword whatever which allows the user to set a property to json "null" to send null as part of the launch/attach request.

Ramblings on which symbol to use In lisp `nil` is `false` and `t` is `true`. But when parsing json "null" and "false" is valid values but there is no obvious to translate into in lisp.

In json-parse-buffer the default value is :null, in jsonrpcit's 'nil and "false" is :json-false.

Not a big fan of any of them as it's not really line up with point (2.) under Solutions but that does not rule out the solution.

One benefit of having an null value is to allow for dape to actually send "null" if an adapter requires null for some reason.

Ignore symbol

Introduce an symbol, let's call it ignore which just removes the property at before sending it off to adapter.
And using as follows with the dape command: gdb :env ignore

Extremely unobtrusive option.

Add an special value construct with docs

This will only solve the problem under "Improve discoverability in batteries included configurations". But can be combined with the other options to solve both.

Something like the following:

(gdb
 modes (c-mode c-ts-mode c++-mode c++-ts-mode)
 command-cwd dape-command-cwd
 command "gdb"
 command-args ("--interpreter=dap")
 fn (dape-config-tramp)
 :request "launch"
 :program ("a.out" :doc "Relative or absolut path to program")
 :env (:doc "Environment variables, example value (:DEBUG \"1\")")
 :stopAtBeginningOfMainSubprogram (nil :doc "Set a temporary breakpoint at the program’s main procedure"))

The :doc information can then be displayed in the hint section of the minibuffer.

Rework dape read config

To allow for removal of properties, which basically means that that all properties needs to be presented in some way to be able to remove them.

Pop up buffer

  • Let the dape command complete from the available configs nothing more
  • Add an additional command dape-edit which lets you edit the configuration

Using edit-macro as inspiration and with combination of "Add an special value construct with docs" can show docs as well.

Transient

Maybe transient could be used in some way am not that familiar with transient.

Define an specialized parser for each config

See #44 (comment)

@svaante svaante changed the title Help/input wanted: Changes to dapeconfig format and minibuffer read functionality Help/input wanted: Changes to dape config format and minibuffer read functionality Feb 14, 2024
svaante added a commit that referenced this issue Mar 7, 2024
Surprise this bug was not hit sooner, in plists and vectors nil where
interpreted as json null and not false.  This might break some user
defined configurations which makes #72 more pressing to solve.
svaante added a commit that referenced this issue Mar 8, 2024
@mohkale
Copy link

mohkale commented May 30, 2024

Hi @svaante,

Have you considered maybe using eioio types to represent debugger configurations. You can form a dedicated debugger class for each non-general type and attach documentation in the same way as customization types (I'm not sure but I think you should also be able to read them back if you want to keep them for a completing read interface).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants