ghcup-hs
ghcup-hs copied to clipboard
haskell
Context Menu
Create popUp widget: widget state, widget drawing and widget handler. The widget is pretty straigthfoward
I have to implement the instalation logic still. I am gonna need some help with it. I understand how to pass some advance parameters, but not others. The whole list of parameters is:
- url
- set
- isolate
- force
- configure Args
If we translate that into code
-- |- Change be IsolateDir ... if parameter isolate is set
-- | |- change to True is param force is set.
installGHCBin (GHCTargetVersion lCross lVer) GHCupInternal False [] $> (vi, dirs, ce)
-- |- Is this list for the rest of params??
Nice.
Although it seems this popup is for installation? We want it for ghcup compile hls.
Although it seems this popup is for installation? We want it for
ghcup compile hls.
Ouch! I got confuse in the conversation. If the new feature is for ghcup compile, how would the user interface be? Another hot-key?
Although it seems this popup is for installation? We want it for
ghcup compile hls.Ouch! I got confuse in the conversation. If the new feature is for
ghcup compile, how would the user interface be? Another hot-key?
I think we're going to run out of hotkeys.
I suggest something like "context menu", maybe defaulting to Enter. Then it has select menu which we may extend in the future.
I suggest something like "context menu", maybe defaulting to
Enter. Then it has select menu which we may extend in the future.
I think I am missing some ghcup knowledge here. From my understanding ghcup compile is a command like ghcup install, or ghcup set. So, Is the "context menu" in fact a "compile menu"? I mean, instead of adding a hot-key for compile, we have a popup with a big "compile" button and a buch of options (following ghcup compile hls --help docs). Maybe there is something about compile I am missing...
For reference I've tried to implement this comment ideas.
So, Is the "context menu" in fact a "compile menu"?
Let's say the context menu for now will have only one button that leads you to said compile menu.
We can add more stuff as we go.
We can add more stuff as we go.
I am playing around with this. Would it be useful to add an small Menu widget utilities? It would be some default widgets for easily creating/composing menus. I am thinking three options data MenuEntry = Button | EditBox | CheckBox. Then you defined your menu as
-- Missing internal state bits for each variant
data MenuEntry = Button | EditBox | CheckBox
-- oversimplification
data CompileMenu = CompileMenu {gitRefEditBox :: MenuEntry, setCheckBox :: MenuEntry, okButton :: MenuEntry}
newCompileMenu = CompileMenu EditBox CheckBox Button
Yeah, why not. We can always change things. Code decisions here are low impact, because they can be easily reverted.
It's harder with our decisions on how to organize installed files. They are hard to revert.
Hi @lsmor
Thank you for your efforts in getting this implemented!
I want to help you get this over the finish line.
To achieve this, I think we first need a clear specification of exactly what it is we want to implement. I have read the linked issue (https://github.com/haskell/ghcup-hs/issues/706) as well as the comments in this PR, and below is my suggestion for a final spec (for this PR):
- A new "context menu" is added
- This is a small popup widget (overlaying part of the main TUI widget) that appears when the user presses ENTER on the currently selected tool (GHCup/Stack/HLS/cabal/GHC) in the TUI
- The title of this popup widget is
Context menu (<tool name> <tool version>), for exampleContext menu (GHC 8.10.7) - The widget contains one or more actions relevant to the given tool. Each action is a line that can be navigated to using the up/down keys and entered by pressing ENTER. All configuration for the given action happens in the widget that appears after pressing ENTER on an action (to keep the context menu clean).
- Pressing ESCAPE closes the context menu (thus showing the default TUI view)
- Generally speaking, this context menu will contain additional actions relevant to the selected tool in question. Thus, the content of the context menu may differ depending on which tool is selected. For example:
- For GHC only there may be a "compile HLS for this GHC version"-item
- For all tools:
- An "advanced installation" item, which allows configuring advanced options such as "isolated install"/"force install"/"compile instead of downloading bin-dist" (the options displayed by
install --help) - A "compile" item, which acts as a TUI for the
ghcup compilecommand
- An "advanced installation" item, which allows configuring advanced options such as "isolated install"/"force install"/"compile instead of downloading bin-dist" (the options displayed by
- For HLS only we can imagine being able to deselect plugins
Example context menu for GHC 8.10.7:
┌────────────────────────────────────────GHCup────────────────────────────────────────┐
│ Tool Version Tags Notes │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
│ ┌───────────────────Context menu (GHC 8.10.7)───────────────────┐ │
│ │┌────────┐ │ │
│ ││Compile │ │ │
│ │└────────┘ │ │
│ │┌─────────────────────┐ │ │
│──────────││Advanced installation│ │──────────│
│ │└─────────────────────┘ │ │
│ │┌────────────────────────────────────────┐ │ │
│ ││Compile HLS for this GHC version │ │ │
│ │└────────────────────────────────────────┘ │ │
│ └───────────────────────────────────────────────────────────────┘ │
│ │
│ │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
└─────────────────────────────────────────────────────────────────────────────────────┘
q:Quit i:Install u:Uninstall s:Set c:ChangeLog a:Show all versions ↑:Up ↓:Down
h:help
Note that the above is only about the nature of the "context menu", which allows putting more advanced features into the TUI without cluttering the main view and adding additional hot keys.
Does this sound reasonable? @hasufell @arjunkathuria @david-christiansen @Kleidukos
If we agree on this, then it's up to you @lsmor to decide which of the above context menu features you want to implement. It looks like you've started to implement the "advanced installation"-item feature. The goal of this PR can then be adding (1) the context menu itself; and (2) whatever context menu feature @lsmor wants to implement.
If @lsmor wants to continue with the "advanced installation"-item feature then a subsequent PR can add the "compile HLS for this GHC version"-feature.
Code decisions here are low impact, because they can be easily reverted.
Well, given the amount of features for this (I wont implement all of them in this PR though), I think It is worth to split the brick related stuff into another component (say lib-tui), instead of having a single file. So I am taking this quote literal, and do It. The only thing I wonder is if this is going to break CI for some reason. In principle I am not planning to break any cabal flag so I guess there should be no problem
I want to help you get this over the finish line.
Thanks, actually I had somethings wrong. This design helped to get things clearer
Thanks, actually I had somethings wrong. This design helped to get things clearer
Great! I'm happy to hear that.
I have to implement the instalation logic still. I am gonna need some help with it. I understand how to pass some advance parameters, but not others. The whole list of parameters is:
- url
- set
- isolate
- force
- configure Args
If we translate that into code
-- |- Change be IsolateDir ... if parameter isolate is set -- | |- change to True is param force is set. installGHCBin (GHCTargetVersion lCross lVer) GHCupInternal False [] $> (vi, dirs, ce) -- |- Is this list for the rest of params??
I'm not that familiar with the GHCup codebase, but after looking around a bit, I can see that all the relevant options are contained in the data type InstallOptions (in the GHCup.OptParse.Install module).
So I would prefer to go a few levels up from installGHCBin, and calling GHCup.OptParse.Install.install. This takes an InstallOptions as argument and ultimately calls installGHCBin. This should also work for all tools instead of just GHC, which we need because the Advanced installation action should be in the Context Menu for all tools.
Generally speaking, I prefer to call some sort of eval function which takes a single data type as argument that specifies everything to do. This avoids having to call specific functions with specific arguments in the right place. In this case the install function seems to behave this way along with the InstallCommand data type.
@hasufell does the above sound reasonable to you? I'm not familiar with how various options are passed around by GHCup, so calling GHCup.OptParse.Install.install from an action within the Context menu might be problematic.
@hasufell does the above sound reasonable to you? I'm not familiar with how various options are passed around by GHCup, so calling
GHCup.OptParse.Install.installfrom an action within the Context menu might be problematic.
I'm not sure I agree. GHCup.OptParse.Install.install was written for the cli interface and assumes cli when doing error handling, logging, etc.
My approach in such cases, where it's not clear what the design overlap is, usually is to rather duplicate code and let us explore what actually ends up similar. Abstraction is, imo, discovered and not actively sought.
Here I'm not sure what a good design is that abstracts over cli and tui. A pre-existing interface may just skew your creativity when coming up with a good API for the tui to consume.
Also remember that this creates coupling and refactors over the optparse code would also affect the tui.
That said, I'd probably expect a small common denominator between cli and tui in a separate sublibrary or the main library, if one really wants to avoid code/logic duplication. So you might end up with 3 ADTs:
- TUI ADT
- opt-parse ADT
- GHCup library ADT, which the other two are converted to
And now I'm not sure it's worth it. But feel free to give it a try.
@hasufell thank you for your input! You make several good points.
And now I'm not sure it's worth it.
I agree. Let's go with the simple solution for now.
@lsmor I will try again to answer your question — please ignore my comment above :-)
- url
- set
- isolate
- force
- configure Args
If we translate that into code
-- |- Change be IsolateDir ... if parameter isolate is set -- | |- change to True is param force is set. installGHCBin (GHCTargetVersion lCross lVer) GHCupInternal False [] $> (vi, dirs, ce) -- |- Is this list for the rest of params??
As you've figured out, the parameters isolate, force and configure args are all arguments to installGHCBin.
If the url option is set (the CLI option called instBindist), then we call installGHCBindist instead of installGHCBin as is done here: https://github.com/haskell/ghcup-hs/blob/df192ee18e64b6d080adc14f498d97853274a543/lib-opt/GHCup/OptParse/Install.hs#L342-L348 and we also run the Excepts actions with the noVerify setting set to True: https://github.com/haskell/ghcup-hs/blob/df192ee18e64b6d080adc14f498d97853274a543/lib-opt/GHCup/OptParse/Install.hs#L340
For the set option, the following line takes care of this: https://github.com/haskell/ghcup-hs/blob/df192ee18e64b6d080adc14f498d97853274a543/lib-opt/GHCup/OptParse/Install.hs#L349
Let me know whether this clears things up.
I am force pushing a commit with a big (but easy) change. I have moved the BrickMain.hs into its own library and split it in many modules. The original BrickMain.hs remains as in master branch. The reasons for this are:
- If we are willing to implement a few menus with advance options, one module becomes too much of a mess to handle
- If we gonna need more than one module, a library becomes a better approach (IMO) than many modules in an executable component
- (selfish reason) This wont be an easy PR, hence a lot of rebase / conflicts are expected. I am willing to re-implement changes in BrickMain.hs into the new library, but rebasing 10 commits each with many conflicts is just painfull (not a commit expert myself)
If any of you have concerns about the commit please ask. In general what I've done is to move "sections" from BrickMain.hs into modules in lib-tui. I have changed some types in order to break cyclic dependencies (but all changes are minor)
@lsmor I think this kind of change makes a lot of sense. Separating into smaller modules increases readability and the sub-library increases testability.
I don't have enough knowledge of the codebase to judge whether this particular organization of modules is the "right" one, but IMO if it works then it's good enough.
@hasufell what do you think about merging this as-is (without the "context menu"-changes)? It works for me, and I think it's a useful change.
Hi there!
I'll be pushing some funcitonality soon. For now I've implemented only the visual part, without logic. Before continuing I'd like to have some feedback. Each tools has its own "Context Menu" (I'd prefer advance options, honestly), from there you can go to other settings like "compile", "install", etc...
Code is very messi, I am cleaning it a liitle bit before pushing.
I'll be pushing some funcitonality soon. For now I've implemented only the visual part, without logic. Before continuing I'd like to have some feedback. Each tools has its own "Context Menu" (I'd prefer advance options, honestly), from there you can go to other settings like "compile", "install", etc...
@lsmor awesome! Looks great.
I would make some small changes — provided it's not too difficult:
- For the "advanced installation", have a help text (ie. description) of each argument. For example:
- what does "force" mean; and
- what am I supposed to put in the "isolated" text field?
- (the help texts are available here: https://github.com/haskell/ghcup-hs/blob/55030d83da6d59196df28c0eb115cdf19ff20393/lib-opt/GHCup/OptParse/Install.hs#L175-L210)
- Include tool version in Context Menu title
- Press ESC to go back to the main screen instead of having a "Cancel" button
- Show this in the Context Menu window in the same way keyboard shortcuts are shown in the main window, e.g.
ESC:Back
- Show this in the Context Menu window in the same way keyboard shortcuts are shown in the main window, e.g.
For the "advanced installation", have a help text (ie. description) of each argument. For example:
I think it should be possible to put that help text inside the input fields (if said input field is not focused). I think that's similar to how many web input fields work and saves space.
Press ESC to go back to the main screen instead of having a "Cancel" button
Maybe the same as in the main window, which by default is q. I think I avoided Esc, because it's more likely to be defined for the terminal window or something. It can still be set by the user to this key.
Otherwise I think this goes into the right direction.
@lsmor let us know if you need any further feedback for the design. I believe this is a major improvement to the TUI.
I am pushing (only) the visuals after rebasing. TODOS:
- [x] migrate the fix #987 to the new library
- [x] add option's help message in grey font
- [x] implement the visual for CompileMenu
- [x] implement the logic for AdvanceInstallMenu
- [x] implement the logic for CompileMenu
- [x] Solve bug about
qused for exit (see below)
@lsmor what's the timeline you think you will have this finished? I'm evaluating whether to wait for it before making a release or doing a release sooner than later.
@lsmor what's the timeline you think you will have this finished? I'm evaluating whether to wait for it before making a release or doing a release sooner than later.
um... I don't expect it to have it soon. the first two points are easy, and I can have them in the next week, but I still don't know how difficult will be to have the last two point working. + There should be some extensive testing which I don't know how to approach actually. So I would expect to have something testable in about 1,5 month. From there, fixing all possible bugs, etc... let say 2 or 3 months to merge.
Btw, I am terrible at ETA. Notice #850 , took me 5+ months (summer in the middle) and I estimated 3 months (and it was merge with a bug on my side!!!)... Also I am more familiar with the code base now.
I am terrible at ETA
Well, I'm not trying to be a manager :D
Just trying to understand when to schedule releases. I think I'll start setting up a milestone then and only merge smaller stuff.
Maybe the same as in the main window, which by default is
q
I just recall, why this isn't a good Idea. You can either type q on a textbox, or q to exit. You could be intelligent enough and dispatch the q depending on whether or not a text box is focused, but thats brittle as it literally depends on pattern matching order... Also, it feels a little bit clumsy.
3. Press ESC to go back to the main screen instead of having a "Cancel" button
AS @hasufell said, adding a different key for exit, is risky as something like ESC might be dispatch to the terminal itself instead of the content within the terminal (example: Ctrl + Enter doesn't work on my gnome's terminal, probably because some colission with terminal's shortcuts)
¿is it ok if we keep the cancell button?
I think it should be possible to put that help text inside the input fields
I am adding this to the check list
I just recall, why this isn't a good Idea. You can either type q on a textbox, or q to exit. You could be intelligent enough and dispatch the q depending on whether or not a text box is focused, but thats brittle as it literally depends on pattern matching order... Also, it feels a little bit clumsy.
I'm not sure I follow. It's very normal that the semantics of key-presses changes depending on which window/box is focused. That's literally how window managers work.
We could make ctrl+c be the default for "exit everything".
It's very normal that the semantics of key-presses changes depending on which window/box is focused
Sure!, so it does in ghcup. Let me ilustrate the problem better.
┌────────────────────────────────────────GHCup────────────────────────────────────────┐
│ Tool Version Tags Notes │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
│ ┌───────────────────Compile (GHC 8.10.7)────────────────────────┐ │
│ │┌────────┐ ┌────────────────────────────────┐ │ │
│ ││isolate │ │ │ │ │
│ │└────────┘ └────────────────────────────────┘ │ │
│ │┌────┐ ┌─────┐ │ │
│──────────││set │ │ X │ │──────────│
│ │└────┘ └─────┘ │ │
│ │ . │ │
│ │ . │ │
│ │ . │ │
│ └───────────────────────────────────────────────────────────────┘ │
│ │
│ │
│─────────────────────────────────────────────────────────────────────────────────────│
│ │
└─────────────────────────────────────────────────────────────────────────────────────┘
q:Quit i:Install u:Uninstall s:Set c:ChangeLog a:Show all versions ↑:Up ↓:Down
h:help
In the example above arrow keys are dispatched to the upfront layer, which is the focused on. That's the expected behaviour. The problem comes when we are over isolate field and we press q; it should write q on the text box, but, If we are over set field, it should exit the menu. Aside from the code complexity and brittleness of it, the user experience is a bit unintuitive.
As for now, I'll continue to implement the missing features (this isn't an stopper). There is a known bug that q exits even if we are over a textbox. I'll add that bug to the checklist
We could make ctrl+c be the default for "exit everything".
If the user changes that to a simple letter we are in the same situation
Rebased + Help messages + migration of #987. I think help messages do look cool. Slow but steady progress.
Small update. It turns out that the API I created for Menus is very very bad. In particular, it can't validate fields with good error messages, and also is very unsafe and error prone. That led to very buggy code, and eventualy I decided to refactor the whole Menu API. I am using Brick.Form as the inspiration for the new API (we can't use it directly because of the use of microlenses instead of optics)
(unrelated disclaimer: I don't know what's going on with CI. It builds in my local computer...)
Hi there.
I am stuck with the ghcup code base. I am implementing the Advance install functionality. I think I am running the same code (or equivalent code) as in ghcup-optparse, but I am getting a weird error.
to reproduce:
- Install a tool isolated via cli. ex:
ghcup install stack 2.15.1 --isolated /some/path/to/test -
stackbinary should be in that directory. Delete it. - Do the same via tui.
- go to the same tool
- press
Enter - Open the install Menu
- fill the isolated field with
/isolated/some/path/to/test - press
Enteron the button on the top
Interestingly the same log is outputed, but it fails with
Error: [GHCup-00070] Unable to copy a file. Reason was: /path/to/test
/stack: openFd: already exists (File exists)
I don't think this is due to installing via cli first, as I can run ghcup install stack 2.15.1 --isolated /some/path/to/test multiple times as long as I delete /some/path/to/test/stack before.
@hasufell do you have any intuition about this?
For completness. Here are both outputs
> ghcup install stack 2.15.1 --isolate /home/luis/delete
[ Info ] downloading: https://raw.githubusercontent.com/haskell/ghcup-metadata/master/ghcup-0.0.8.yaml as file /home/luis/.ghcup/cache/ghcup-0.0.8.yaml
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
[ Warn ] New ghc version available. If you want to install this latest version, run 'ghcup install ghc 9.8.2'
[ Info ] downloading: https://downloads.haskell.org/~ghcup/unofficial-bindists/stack/2.15.1/stack-2.15.1-linux-x86_64.tar.gz as file /home/luis/.ghcup/tmp/ghcup-6148c2bcd44f02bb/stack-2.15.1-linux-x86_64.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 20.5M 100 20.5M 0 0 14.3M 0 0:00:01 0:00:01 --:--:-- 14.3M
[ Info ] verifying digest of: stack-2.15.1-linux-x86_64.tar.gz
[ Info ] Unpacking: stack-2.15.1-linux-x86_64.tar.gz to /home/luis/.ghcup/tmp/ghcup-20341ce62eefc284
[ Info ] isolated installing Stack to /home/luis/delete
[ Info ] Installing stack
[ Info ] Stack installation successful
[ Info ] Stack manages GHC versions internally by default. To improve integration, please visit:
[ ... ] https://www.haskell.org/ghcup/guide/#stack-integration
[ ... ]
[ ... ] Also check out:
[ ... ] https://docs.haskellstack.org/en/stable/yaml_configuration
[ ... ]
> rm /home/luis/delete/stack
> cabal run ghcup -- tui
... # do whatever on tui
[ Info ] downloading: https://downloads.haskell.org/~ghcup/unofficial-bindists/stack/2.15.1/stack-2.15.1-linux-x86_64.tar.gz as file /home/luis/.ghcup/tmp/ghcup-8dab1a41a66d3f3d/stack-2.15.1-linux-x86_64.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 20.5M 100 20.5M 0 0 14.7M 0 0:00:01 0:00:01 --:--:-- 14.7M
[ Info ] verifying digest of: stack-2.15.1-linux-x86_64.tar.gz
[ Info ] Unpacking: stack-2.15.1-linux-x86_64.tar.gz to /home/luis/.ghcup/tmp/ghcup-70ce6210c3c83de3
[ Info ] isolated installing Stack to /home/luis/delete
[ Info ] Installing stack
Error: [GHCup-00070] Unable to copy a file. Reason was: /home/luis/delete
/stack: openFd: already exists (File exists)
Also check the logs in ~/.ghcup/logs
Press enter to continue