Skip to content

feat: run/debug profile (#110, #131)#146

Merged
s1n7ax merged 4 commits intonvim-java:mainfrom
atm1020:feature-profile
Apr 12, 2024
Merged

feat: run/debug profile (#110, #131)#146
s1n7ax merged 4 commits intonvim-java:mainfrom
atm1020:feature-profile

Conversation

@atm1020
Copy link
Copy Markdown
Contributor

@atm1020 atm1020 commented Mar 22, 2024 •
edited
Loading

Since nvim.nui is already in use, I focused on creating a small UI with it for creating, editing, and changing the active profile.

profile.mp4

Profile settings options:

  • vm args
  • prog args

Profile ui

  • Open with JavaProfile command
  • Create Profile
    • Select the new profile option to create a new one (press enter/space)
    • Press s in normal mode to save.
    • The new profile is set active by default.
  • Update
    • Select profile to edit (press enter/space)
    • Press s in normal mode to save.
  • Set Active
    • Press a in normal mode in a list

All Key binding (in normal mode) :

  • Profile List:
    • j: Down
    • k: Up
    • space: Edit
    • enter: Edit
    • b: Quit/Back
    • q: Quit
    • a: Set Active
    • d: Delete
  • Profile Edit:
    • b: Back
    • q: Quit
    • s: Save

The profile config is saved to a JSON file named nvim-java-profiles.json in the ~/.local/share/nvim folder. Saving to the project folder to the .config.json file or something like that, similar to what IntelliJ does, is still a possible option, but for dev/testing, one config feels more manageable.

json structure:

{
 "project_folder": {
    "profile_name1": {
      "vm_arg": "Vm-arg",
      "prog_arg": "Prog-arg",    
      "profile_name": "profile_name1"
 }
}

@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Mar 24, 2024

Draft because still being worked on?

@atm1020
Copy link
Copy Markdown
Contributor Author

atm1020 commented Mar 24, 2024 •
edited
Loading

@s1n7ax I marked the pr as a draft because the tests are missing, and i expect some change requests, so i want to avoid rewriting the tests.

@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Mar 24, 2024

@atm1020 I see, I will review it ASAP

s1n7ax
s1n7ax requested changes Mar 26, 2024
View reviewed changes
@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Mar 26, 2024

@atm1020 There is one simple change I would like. Fabulous work though. Make sure to post your work in reddit once implemented.

@atm1020
Copy link
Copy Markdown
Contributor Author

atm1020 commented Mar 28, 2024

@s1n7ax Thank you for your kind words! I will try to finish in the next few days

@atm1020 atm1020 force-pushed the feature-profile branch 3 times, most recently from 558768c to f5ac95b Compare April 1, 2024 10:09
@atm1020
Copy link
Copy Markdown
Contributor Author

atm1020 commented Apr 1, 2024

@s1n7ax I think i've finished it.

  • As you requested, I switched to the Penlight class (additionally added it to the runner API. as well).
  • I made some minor restructuring in "ui.profile" and "api.profile_config" files
  • I found an issue when you making changes to the active profile or creating a new one while a DAP session is active doesnt apply the new configuration with a simple session restart (The session has to be terminated to apply the new config).
    To avoid this confusion, I currently terminate the current DAP session before reconfiguring.

bug or question :

  • When adding a new profile, if the jdtls client isn't initialized yet, the dap update fails. (The dap update runs after the profile is saved successfully so there is no missing data). However, an error message appears about the jdtls client, which can be confusing. If you think this needs fixing, we could check if the jdtls client is initialized before saving the profile.

atm1020 added 2 commits April 1, 2024 13:08
@atm1020
feat: profile (nvim-java#110, nvim-java#131)
dde2149 
add project specific profile
add ui for profile setup
pass vm and prog ars to running/debug api
@atm1020
feat: add penlight class, tests, and some minor restructuring.
947ea95
@atm1020
Copy link
Copy Markdown
Contributor Author

atm1020 commented Apr 1, 2024

@s1n7ax
I think about the b key for "go back" actions in normal mode, it might not be the best choice because it overrides the default vim command ( move back one word) . What do you think shold i change to bb or something, or enable user configuration options for all the key bindings (may be it's little bit out of scope right now, but I'm not sure) ?

s1n7ax
s1n7ax requested changes Apr 2, 2024
View reviewed changes
@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Apr 2, 2024

@atm1020 I think it's better to add the shortcut details below the window. Something like lazygit does. I really couldn't figure out how to save the profile.

In the future we can have a ? for another popup with all keymaps

image

@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Apr 2, 2024

When adding a new profile, if the jdtls client isn't initialized yet, the dap update fails

Good enough for now. Might be possible to add onetime autocmd to do that on server setup.

Right now configuration can be accessed using vim.g.nvim_java_config from anywhere. We can make the keys customizable in the future. These defaults are fine by me for now.

@atm1020
Copy link
Copy Markdown
Contributor Author

atm1020 commented Apr 2, 2024 •
edited
Loading

@atm1020 I think it's better to add the shortcut details below the window. Something like lazygit does. I really couldn't figure out how to save the profile.

In the future we can have a ? for another popup with all keymaps

image

@s1n7ax I set up two options for it, what do you think, which one is better?

  1. With border config
    image
    image

  2. With Layout + Popup
    image
    image

@s1n7ax
Copy link
Copy Markdown
Member

s1n7ax commented Apr 3, 2024

@atm1020 1st one looks cool.

s1n7ax
s1n7ax requested changes Apr 3, 2024
View reviewed changes
@atm1020
feat: handle multi module projects
8699703 
add navigation keymaps
s1n7ax
s1n7ax approved these changes Apr 12, 2024
View reviewed changes
@s1n7ax s1n7ax marked this pull request as ready for review April 12, 2024 03:12
@s1n7ax s1n7ax merged commit 3442006 into nvim-java:main Apr 12, 2024
@github-actions github-actions bot mentioned this pull request Apr 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1n7ax s1n7ax s1n7ax approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@atm1020 @s1n7ax