Skip to content

string repeat: allow omission of -n#10282

Merged
henrikhorluck merged 2 commits into
fish-shell:masterfrom
lavafroth:string-repeat-n
Feb 11, 2024
Merged

string repeat: allow omission of -n#10282
henrikhorluck merged 2 commits into
fish-shell:masterfrom
lavafroth:string-repeat-n

Conversation

@lavafroth

Copy link
Copy Markdown
Contributor

Description

string repeat will expect the first positional argument as the count if it is not supplied through the -n flag.

Fixes issue #9332

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@zanchey zanchey requested review from krobelus and mqudsi February 6, 2024 04:03
@krobelus

krobelus commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

this history looks more complicated than necessary, can you simplify it (remove merge commits, squash commits as appropriate)

@lavafroth

lavafroth commented Feb 7, 2024 via email

Copy link
Copy Markdown
Contributor Author

@zanchey zanchey requested review from henrikhorluck and removed request for krobelus and mqudsi February 7, 2024 08:43

@henrikhorluck henrikhorluck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I found a minor behaviour change that we can avoid, and some minor code style nits, but otherwise LGTM!

This needs relnotes and documentation updates in CHANGELOG.rst, doc_src/cmds/string-repeat.rst and doc_src/cmds/string.rst. The signature change is in the linked issue so should be straightforward. I can deal with that if you would prefer not to.

Comment thread src/builtins/string/repeat.rs Outdated
Comment thread src/builtins/string/repeat.rs Outdated
Comment thread tests/checks/string.fish
@lavafroth

lavafroth commented Feb 11, 2024

Copy link
Copy Markdown
Contributor Author

I have committed the suggested changes.
Edit: I haven't made changes to CHANGELOG.rst, doc_src/cmds/string-repeat.rst or doc_src/cmds/string.rst.

@henrikhorluck henrikhorluck merged commit 4e6e897 into fish-shell:master Feb 11, 2024
@henrikhorluck

Copy link
Copy Markdown
Contributor

Merged, thanks!

@zanchey zanchey added this to the fish next-3.x milestone Feb 11, 2024
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants