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

feat: extend multiselection prompt with pagesize option (#65) #66

Conversation

groma84
Copy link

@groma84 groma84 commented Aug 3, 2023

I extended MultiSelectionPrompt with MultiSelectionPromptOptions to allow setting the PageSize

I could not actively test it because the freshly cloned main branch does not build sadly.

@EluciusFTW
Copy link
Owner

Hi @groma84, thanks for the contribution! We just pushed/merged a PR fixing the build. If you rebase now you can test your changes.

It happened due to some simultaneous changes from both of us. We'll ad a CI build action soon to prevent this in the future!

src/spectrecoff/Prompt.fs Outdated Show resolved Hide resolved
src/spectrecoff/Prompt.fs Outdated Show resolved Hide resolved
src/spectrecoff/Prompt.fs Show resolved Hide resolved
src/spectrecoff-cli/commands/Prompt.fs Outdated Show resolved Hide resolved
@groma84
Copy link
Author

groma84 commented Aug 7, 2023

Thanks for all the feedback - I'll work on the PR again on Thursday and will include all your suggested improvements!

@groma84 groma84 force-pushed the groma84/extend-multiselection-prompt-with-pagesize-option--#65 branch from 34cb171 to af11adb Compare August 20, 2023 09:56
@groma84
Copy link
Author

groma84 commented Aug 20, 2023

I implemented the suggestions but I still can't build the -cli project locally, even after rebasing: (sorry for the German messages, basically it says Count and ToArray aren't defined)

...\SpectreCoff\src\spectrecoff-cli\commands\Prompt.fs(19,28): error FS0039: Das Feld, der
Konstruktor oder der Member "Count" ist nicht definiert. [C:\workspace\groma84@github.com\SpectreCoff\src\spectrecoff-c
li\SpectreCoff.Cli.fsproj]
...\SpectreCoff\src\spectrecoff-cli\commands\Prompt.fs(22,44): error FS0039: Das Feld, der
Konstruktor oder der Member "ToArray" ist nicht definiert. [C:\workspace\groma84@github.com\SpectreCoff\src\spectrecoff
-cli\SpectreCoff.Cli.fsproj]
...\SpectreCoff\src\spectrecoff-cli\commands\Prompt.fs(25,53): error FS0039: Das Feld, der
Konstruktor oder der Member "Count" ist nicht definiert. [C:\workspace\groma84@github.com\SpectreCoff\src\spectrecoff-c
li\SpectreCoff.Cli.fsproj]

@EluciusFTW
Copy link
Owner

Hi Martin,
first of all, German is fine, we both actually are German native speakers :)

I checke out your branch to investigate, and this time, unfortunately (or is it, hehe) the build error is due to your changes. As you can see in this screenshot I checked out two commits backwardds on your branch to the main state, and the build succeeds:

image

I guess the type inference is now defaulting to Seq instead of List on one of the underlying methods. Maybe you need to add some types nonetheless. I did a quick fix and pushed it to the branch. Feel free to rework the commit, if you think I overstepped!

@groma84
Copy link
Author

groma84 commented Aug 20, 2023

Thanks for the fix!
VSCode with Ionide did not show me anything at all for the projects, thus I had a hard time figuring the issue out.
So thanks for not only pointing me in the right direction but also fixing it!

@EluciusFTW EluciusFTW merged commit 651cfdb into EluciusFTW:main Aug 22, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants