Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

Discussion of a few modifications #5

Open
in0ni opened this issue Jul 22, 2022 · 7 comments
Open

Discussion of a few modifications #5

in0ni opened this issue Jul 22, 2022 · 7 comments

Comments

@in0ni
Copy link
Contributor

in0ni commented Jul 22, 2022

Hello,
I'm hoping to use this ticket as a platform to discuss a couple of things I have been thinking contributing with. One I have already started work on as it affects my workflow. There are also a feature request, I wanted to know if can be done.

Using default clients for variables/stacktrace

I find that the dap-setup-ui and dap-takedown-ui are perfect for customizing layouts for personal preferences. Though using the clients kakoune already provides might be best for the default behavior: toolsclient and docsclient. Kakoune and most plugins that require output use these. For example:

  • grep/find
  • documentation
  • linters
  • kak-gbdp

In my case I wrote a script that builds a sort of IDE, based on this: https://github.com/mawww/kakoune/wiki/IDE. So I have my environment setup and the three clients are always used. I have already created a local branch which:

  • has a boolean option that the user can set to determine if they want to use the two provided commands (false by default)
  • uses docsclient and toolsclient for both vars/stacktrace
  • for now I simply commented out takedown-ui in the general.rs (but will handle it better)

Stacktrace syntax and opening files

I think I will take the time for syntax highlighting, and the kakoune-find plugin can be used as a reference for opening the files in the jumpclient

Don't show "+" arrow on empty arrays

This is minor, if I can tackle it I will. You can see in it in the screenshot for my pull request for variable syntax hl. While testing I thought something was broken, but it was due to me clicking on the array(0) variables.

Feature request

Is it possible to toggle breakpoints after the debugging session has started?

Please let me know your thoughts. They are minor things, and I can definitely contribute on the first three -- definitely going to wrap up the first one soon and can provide a pull request. Just wanted to know your thoughts.

@jdugan6240
Copy link
Owner

Hello again! I'm just going to address your points one by one.

Using default clients for variables/stacktrace - Don't submit a pull request for this. This can be handled without any changes to kak-dap. The dap-setup-ui, dap-takedown-ui, and dap-run-terminal commands are designed to be overridden for this very reason. For your use case, try putting the following commands (warning: untested) into your kakrc:

# This just sets the stacktrace and variables clients to the docs and "tools clients, respectively.
define-command -override dap-setup-ui %{
    set global stacktraceclient docsclient
    set global variablesclient toolsclient
}
# This does absolutely nothing. We want to keep the clients open for later use by other plugins.
define-command -override dap-takedown-ui %{}

More information about modifying kak-dap's layout can be found here: https://codeberg.org/jdugan6240/kak-dap/wiki/Customizing-debugger-UI.

Stacktrace syntax and opening files - If you want to submit a pull request implementing this, absolutely - go for it. That sounds legit awesome.

Don't show "+" arrow on empty arrays - I don't think this is possible, to be completely honest, and here's why.

Every variable object returned by the debug adapter has a variables_reference parameter. If this is zero, then the variable isn't expandable. If it is greater than zero, then it is expandable. This value is used by child variables to indicate which variable is its parent. This is unfortunately the only way kak-dap can know to place the + sign next to a given variable. Look through lines 104-134 of variables.rs if you want a better grasp of the overall logic.

Long story short, if the adapter is returning a variables_reference value that isn't zero for a variable that can't be expanded, that seems to be a bug with the adapter - the variables_reference value is our only clue for if a variable is expandable.

Feature request - It is indeed possible to implement this, but it is difficult due to how the DAP is designed.

Basically, when sending breakpoints to a debug adapter, you can't just set or remove a single breakpoint - you have to update all the breakpoints in any given file in a single message. This means that, whenever a breakpoint is set or removed at runtime, we would need to remember what breakpoints are associated with that file, and set or remove the breakpoint in that group of breakpoints, then send those breakpoints to the adapter all at once. It's frankly extremely over-engineered, but it's what we've got to work with.

In short, it's very possible to do this, but it's far from trivial, and the "you can't set breakpoints at runtime" restriction was just me kicking the can further down the road. I do plan to tackle this at some point, though.

@in0ni
Copy link
Contributor Author

in0ni commented Jul 23, 2022

@jdugan6240,

Thanks for taking your time to read my long message, hah. All sounds good. I might take a look at the breakpoint issue now that you have explained it, just out of curiosity to see if I can help out.

Regarding the default client -- I fully understand how it's currently setup, and have read the docs. I realize now that what I describe in my approach is not best. I am not suggesting to change the feature, just the default behavior.

I highly recommend DAP should use the docsclient, and toolsclients by default. This is pretty standard, I mention in the original post just a few examples of this being the case.

That being said, what I propose is changing the dap-setup-ui and dap-takedown-ui to use default clients -- and allow users to customize if they want to use custom clients -- not the other way around. Currently one has to write custom code if they want to use the default clients, which is odd to be honest. Think about it, if you use the linter, find, and grep -- and each opens their own custom client, that would be quite frustrating.

The point here is to use default clients, as many other features/plugins do -- and allow a user to customize if they want something non-standard. I hope this makes sense, don't think I am suggesting anything but following the default approach to using the tools/docs clients where possible.

@jdugan6240
Copy link
Owner

jdugan6240 commented Jul 23, 2022

I hear you regarding the default clients. I'm by no means married to the idea of custom clients for kak-dap. I personally don't use toolsclient and docsclient myself too much, and I also didn't believe docsclient in particular fit what we're trying to do here.

My main issue with this is that we shouldn't assume these clients exist if we do change to using docsclient and toolsclient (another question - which would be which?). We don't want to create a dependency on running one of the aforementioned tools to create these clients beforehand, so we may need to create them ourselves. For dap-setup-ui, if they do exist, we wouldn't want to recreate them, whereas if they don't, we would need to create them. For takedown-ui, do we want to delete these clients or leave them up?

There's also the issue of the tools you mentioned defaulting to the same client if there's only one (which I imagine is how many people use Kakoune). This would make the jumpclient and the toolsclient or docsclient the same client, which would cause issues when we need to update both in different ways.

Overall, although I get you wanting to have it work that way for your use case, making it the default raises a bunch of questions that are neatly sidestepped by using custom clients, and if the user wants to use docsclient and toolsclient, doing so is trivial.

Not trying to be combative, by the way - just trying to explain why things are the way they are. I'm certainly open to changing this behavior - I just don't see a good reason to right now.

@in0ni
Copy link
Contributor Author

in0ni commented Aug 1, 2022

Hey, sorry for the very late response

What other commands/plugins do is if the clients don't exist they open on the same client. I'm guessing a use case like working on pure console, a server with limited capabilities. Opening new clients might be an issue, as to not create dependencies, or assumptions on environment.

Regarding the jump and tools client having to update both in different ways, to be honest I am unsure of the underlying behavior. But may be able to look into it to see if it's an issue... time permitting.

My use case is defined by the default behavior of Kakoune, so I don't think this would just apply to me and how I use the editor. It comes with three clients, and various tools/plugins use them. I use docs for vars, tools for stack -- basically I chose docs as I find variables to be reference to the code, and tools as the stacktrace can later be used like grep/find where it can open up code in the jump client... but to be honest, it's just making use of what is already there and not needing to create new ones.

I have absolutely no issue using the exiting commands, but I do feel it's a little backwards to have to modify the default behavior to go along with the default clients that exist, but this is your call really :)

@jdugan6240
Copy link
Owner

OK, I'm starting to understand where you're coming from, I think. Being consistent with the way other plugins work does have a certain appeal. That said, kak-dap isn't the only plugin that uses custom clients like this (kaktree being a notable example of a plugin that does this), and there is another issue, I think, with using the "default" clients:

My use case is defined by the default behavior of Kakoune, so I don't think this would just apply to me and how I use the editor. It comes with three clients, and various tools/plugins use them.

This seems to be a conflict of anecdotes - you heavily use docsclient and toolsclient, whereas I don't use them often at all (and when I do, they're the same as the main client). You don't believe you're alone, and I don't believe I'm alone. These anecdotes seem to me to cancel out, therefore.

The reason I believe the way I do about this issue is, I don't consider what you describe to be the default behavior. Kakoune does not come with three clients (the IDE script is a third party contribution to the wiki, and must be inserted into your config), and as you say, the plugins that ship with Kakoune default to using the same client when docsclient and toolsclient aren't defined (which they aren't, by default).

The big reason using a single client is problematic here is due to how the buffers are updated. kak-dap uses kak -p invocations to send commands to the correct client, which means the order in which the buffers are updated is at the mercy of the process scheduler and the behavior of Kakoune's command socket. This creates inherent unpredictability, which is avoided with multiple clients. If at all possible, we need to avoid using the same client by default, to avoid an inferior experience for users.

IMO, we simply can't make any assumptions about what environment the user is running, and whether docsclient and toolsclient actually exist. We had to default to something, and this custom clients approach seemed to be the best way to ensure basic functionality for those who just install the plugin without changing their kakrc. We have to guarantee those clients exist, for any environment other than a dumb terminal with no multiplexers or windowing environment installed whatsoever.

Please know that I am not trying to be combative or dismissive of your suggestions - they are well appreciated and I am frankly flattered that someone found my silly experiment worth using. I simply believe that using docsclient and toolsclient by default, like other plugins do, opens a can of worms that I'd rather not open by default. The default works to varying degrees for every scenario other than a dumb terminal, and changing the default behavior is a trivial task if necessary.


If you don't mind, let's get off this tangent and start discussing some of your other suggestions. I had been thinking for a while of doing a thorough rewrite of the Rust side of kak-dap, now that I have a better understanding of how Rust works and how to architect a Rust application. This would be a great opportunity to implement setting breakpoints during debugging (I have a pretty good idea of how to approach this). There's also a memory leak regarding variables, which your comment about expanding empty arrays actually reminded me of, that I could fix as well.

This would obviously take some time, as I don't have a ton of free time. This wouldn't touch the kakscript too much, though, which would leave you free to implement the stacktrace stuff if you wish.

I would also like to direct you to pesticide, which is another DAP client designed to work with Kakoune that is in early alpha. It has a different approach to things, using a separate terminal window as the debugger window, which eliminates the clients issues we've been discussing. Some inspiration could potentially be taken from there (I'm definitely going to be borrowing ideas from the code).

@in0ni
Copy link
Contributor Author

in0ni commented Aug 11, 2022

Hey, sorry... I've been swamped.

Yeah, also tagbar adds a different window. I think it makes sense for these use cases. Let's just leave it at that and I'll use the recommended approach, not a big deal.

That being said I regularly use the plugin, and works well. Settings breakpoints while debugging would be great! I'm in the same boat, and my contribution here was more out of need to make reading variables/interaction a little easier. So I hope to contribute the stacktrace stuff later.

Interesting, will have to check pesticide out.

Thank again for your work!

@jdugan6240
Copy link
Owner

jdugan6240 commented Mar 20, 2023

Sorry to revive this really old discussion, but I thought you'd want to know that work on this plugin has moved to Sourcehut: https://git.sr.ht/~jdugan6240/kak-dap.

I am in the middle of a complete rewrite of kak-dap in Python, with a better knowledge of how the plugin should work, and am addressing some of the issues you brought up here. For example:

  • Your point about some variables being marked as expandable when they shouldn't have been. Yeah, this ended up being a kak-dap bug. Basically, the "icon" was blank as it should be for any non-expandable variables at first, but if anything expandable showed up after that, the icon would never again update to blank for any future non-expandable variables.
  • Your point about the client usage. Again, the option to manually determine what clients to use will still exist, but the plugin will, out of the box, attempt to determine which windowing environment you're using and set up a layout accordingly.

This rewrite also comes with changes to how you'll configure kak-dap. Basically, the .kak-dap.json file has been completely reworked, now being YAML instead of JSON, and debug adapters are defined in a new adapters.yaml file in the kak-dap directory. The reason for this is I hope to eventually support having the plugin install debug adapters for you with a dap-install command, which should ease getting started with it (and YAML is much nicer as a config language than JSON).

If you want to check it out, check out the the-rewritening branch on the Sourcehut repo. It's just about functional at this point, but is lacking some features and the documentation is basically nonexistent. If you wanted to contribute, questions can be directed to the following mailing list: https://lists.sr.ht/~jdugan6240/kak-plugins-discuss.

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

No branches or pull requests

2 participants