Skip to content

feat: adding mandatory flag to string input#844

Closed
Nico-DF wants to merge 3 commits intospring-projects:mainfrom
Nico-DF:feature/string-required
Closed

feat: adding mandatory flag to string input#844
Nico-DF wants to merge 3 commits intospring-projects:mainfrom
Nico-DF:feature/string-required

Conversation

@Nico-DF
Copy link
Copy Markdown
Contributor

@Nico-DF Nico-DF commented Aug 4, 2023

As I began to use spring-shell for some CLI workflow, I found that there is no input required for string (altough there is default).

For the moment, I used this kind of code:

public String input(String message, String defaultValue, boolean mandatory) {
	var component = new StringInput(getTerminal(), message, defaultValue);
	component.setResourceLoader(getResourceLoader());
	component.setTemplateExecutor(getTemplateExecutor());
	var context = component.run(ConfirmationInput.ConfirmationInputContext.empty());
	var result = context.getInput();

	while (mandatory && !StringUtils.hasText(result)) {
		result = component.run(ConfirmationInput.ConfirmationInputContext.empty()).getInput();
	}

	return result;
}

or this as flow:

componentFlowBuilder.clone().reset() //
    .withStringInput("id")
	.name("Select an identifier")
	.next(ctx -> StringUtils.isBlank(ctx.getInput()) ? "id" : null)
    .and().build();

Which work but it could be simpler, hence my PR.

So I:

  • Added to flow the mandatory flag
  • Added to component the mandatory flag
  • Add simple test case
  • Add samples

Result:
image
image
image

@Nico-DF Nico-DF force-pushed the feature/string-required branch 2 times, most recently from 886336a to 93cd27f Compare August 4, 2023 14:24
@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Aug 8, 2023 •
edited
Loading

Issue with jdk8 compilation doesn't seem to originate from me, right?

(investigating though to be sure)

@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Aug 8, 2023

As suspected I broke something :D, I'll check it

@jvalkeal
Copy link
Copy Markdown
Contributor

jvalkeal commented Aug 8, 2023

I'm just thinking if this is going to be a bit confusing for a user if there's a default value.

my-shell:>component string required
? Enter value [Required][Default myvalue]

I'm kinda questioning if there's better way to give indication to user that something is required.

@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Aug 8, 2023

I'm just thinking if this is going to be a bit confusing for a user if there's a default value.

my-shell:>component string required
? Enter value [Required][Default myvalue]

I'm kinda questioning if there's better way to give indication to user that something is required.

I also wondered about it. I think the easiest way would be to display Required only if there is no default (and it also matches with what really happens)

@fmbenhassine
Copy link
Copy Markdown
Contributor

fmbenhassine commented Oct 30, 2025

@Nico-DF Can you please rebase this PR on the latest main? I will check if we can include it in the upcoming 4.0. Thank you upfront.

@fmbenhassine fmbenhassine added the status/need-feedback Calling participant to provide feedback label Oct 30, 2025
@Nico-DF Nico-DF force-pushed the feature/string-required branch from 28e5d29 to 903d2cf Compare October 30, 2025 11:41
@Nico-DF Nico-DF changed the base branch from 2.1.x to main October 30, 2025 11:41
@Nico-DF Nico-DF force-pushed the feature/string-required branch 2 times, most recently from 6305112 to cfd1619 Compare October 30, 2025 12:20
@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Oct 30, 2025

Update and signing done

@fmbenhassine
Copy link
Copy Markdown
Contributor

fmbenhassine commented Oct 30, 2025

Thank you for the quick updates! You were faster than I expected 😂 I was in the middle of a big refactoring when I asked for a rebase and I didn't think your update would come the same day..

Anyway, I will try to rebase your PR myself, otherwise I will let you know.

@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Oct 30, 2025

No problem, was in between tasks at work, so I had time to do it. I can rebase it tomorrow morning if you want, just ping me when all is finished on main

@fmbenhassine
Copy link
Copy Markdown
Contributor

fmbenhassine commented Oct 31, 2025

Thank you very much! But no rush, we can merge this (as well as #849) in RC1 next week. I will drop a message here when main is stable again (some restructuring is ongoing).

@fmbenhassine fmbenhassine added this to the 4.0.0 milestone Oct 31, 2025
@fmbenhassine
Copy link
Copy Markdown
Contributor

fmbenhassine commented Nov 27, 2025 •
edited
Loading

Hi @Nico-DF

I think we can proceed and merge this PR now. Can you please rebase it on the latest main?

Please exclude the changes in samples, they are outdated.

Thank you upfront.

@Nico-DF
feat: adding mandatory flag to string input
8a2d042 
- Added to flow
- Added to component
- Add simple test case
- Add samples

Signed-off-by: Nico-DF <difalco.nicola@gmail.com>
@Nico-DF
feat: rename mandatory to required
34d6a46 
Signed-off-by: Nico-DF <difalco.nicola@gmail.com>
@Nico-DF
fix: display required only if no default value
db745c6 
Signed-off-by: Nico-DF <difalco.nicola@gmail.com>
@Nico-DF Nico-DF force-pushed the feature/string-required branch from cfd1619 to db745c6 Compare November 27, 2025 14:54
@Nico-DF
Copy link
Copy Markdown
Contributor Author

Nico-DF commented Nov 27, 2025

Done 👍
Not had time to compile locally yet 😋

fmbenhassine pushed a commit that referenced this pull request Nov 28, 2025
@Nico-DF @fmbenhassine
Add mandatory flag to string input
2bdf843 
- Added to flow
- Added to component
- Add simple test case
- Add samples

PR #844

Signed-off-by: Nico-DF <difalco.nicola@gmail.com>
@fmbenhassine
Copy link
Copy Markdown
Contributor

fmbenhassine commented Nov 28, 2025

Thank you for the updates!

Not had time to compile locally yet 😋

I tired locally and it compiles! Ship it 😂 2bdf843

Thank you for your contribution!

@fmbenhassine fmbenhassine added type/enhancement Is an enhancement request and removed status/need-feedback Calling participant to provide feedback labels Nov 28, 2025
@fmbenhassine fmbenhassine added the area/components Belongs ui components label Nov 28, 2025
@fmbenhassine fmbenhassine modified the milestones: 4.0.0, 4.0.0-M2 Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jvalkeal jvalkeal jvalkeal left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/components Belongs ui components type/enhancement Is an enhancement request

Projects

None yet

Milestone

4.0.0-M2

Development

Successfully merging this pull request may close these issues.

3 participants

@Nico-DF @jvalkeal @fmbenhassine