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

A simple way to concat-and-assign (like `foo+=bar`) #1326

Closed
lloeki opened this Issue Mar 4, 2014 · 30 comments

Comments

Projects
None yet
8 participants
@lloeki
Copy link

lloeki commented Mar 4, 2014

Manipulating string ins fish basically requires

A list of things that bash/zsh support that makes everyone life's easier and faster:

  • concat adn assign operator: foo+="bar"
  • pattern in string tests: [ $haystack = *needle* ] (not contains)
  • prefix and suffix (longest and shortest) pattern matching and replacement (reference)

Those largely make calls to sed/grep/awk/basename/dirname useless, thus improving performance a lot in many cases, e.g #1325.

@lloeki

This comment has been minimized.

Copy link

lloeki commented Mar 4, 2014

Linked to #156 I guess.

@xfix

This comment has been minimized.

Copy link
Member

xfix commented Mar 4, 2014

Concatenation already exists. Pattern matching as well, but it requires switch, which cannot be called intuitive (and it's a rather heavy structure).

set foo "$foo"bar
@lloeki

This comment has been minimized.

Copy link

lloeki commented Mar 4, 2014

That is not "concat_and_assign" though, so that gives unwieldy results for non-trivial variables and strings. I'd love to have a set -a foo "bar" to do exactly this (on a string to concat or an array to append a new item).

Compare and contrast this and this, or this and this.

The non repetition of the variable name makes it both more readable and easier to maintain (DRY and PEP8 "readability counts").

I think we can agree on abusing switch/case not being a good idea, especially that in the above linked scenarios it just doesn't work. We may have a solution in #156 for that, but I feel test should be on par with switch for consistency.

@xfix

This comment has been minimized.

Copy link
Member

xfix commented Mar 6, 2014

If you want an append function, there is a sample one. It probably can be improved (for option parsing), but it already works. __fish_value is used, so people could append to value.

function append --no-scope-shadowing
    if test (count $argv) -ne 2
        echo append: Expected two arguments, (count $argv) received.
        return 1
    end
    set -l __fish_value $$argv[1]
    set $argv[1] "$__fish_value$argv[2]"
end

And example, just to show it really works.

xfix@pear ~> function append --no-scope-shadowing
                 if test (count $argv) -ne 2
                     echo append: Expected two arguments, (count $argv) received.
                     return 1
                 end
                 set -l __fish_value $$argv[1]
                 set $argv[1] "$__fish_value$argv[2]"
             end
xfix@pear ~> set test lol
xfix@pear ~> echo $test
lol
xfix@pear ~> append test fail
xfix@pear ~> echo $test
lolfail
xfix@pear ~>

I rarely saw a situation where append would be helpful, and in most cases, people implement complex scripts using other languages, such as AWK or Perl (but let's be honest, AWK doesn't have operator you want).

As for pattern matching, I think something like this can work.

function match
    if test (count $argv) -ne 2
        echo match: Expected two arguments, (count $argv) received.
        return 1
    end
    switch $argv[2]
        case $argv[1]
            return 0
    end
    return 1
end

Example, as always:

xfix@pear ~> function match
                 if test (count $argv) -ne 2
                     echo match: Expected two arguments, (count $argv) received.
                     return 1
                 end
                 switch $argv[2]
                     case $argv[1]
                         return 0
                 end
                 return 1
             end
xfix@pear ~> match '-*' --help; and echo Match.
Match.
xfix@pear ~> match '-*' help; and echo Match.
xfix@pear ~>
@lloeki

This comment has been minimized.

Copy link

lloeki commented Mar 6, 2014

Thanks. Both of those are great. --no-scope-shadowing will be quite useful to me in other cases.

Awk and perl are not useful when you're implementing a rich fish config and prompt, and for performance to be great you have to stay as much as possible in the shell, which pushes its scripting language to the limit. In unscientific tests with the above linked features, all advantages of fish in performance are lost to zsh and even bash solely due to the need to repeatedly call sed.

Allow me to veer a little out of topic. If fish had more 'fringe' features (in terms of the law of minimalism) like this, thus widely increasing the immediate usefulness of the language while only adding a very limited maintenance burden, I would readily use it much more to write scripts than perl or awk. Currently having a choice between bash (useful, piles of historical hacks) and perl/python/awk (more expressive, cleaner) makes me feel there's a need for something in between, and fish has all it takes to fill that void but those 'fringe' features that makes it feel like a minimalist but robust script language.

@ridiculousfish ridiculousfish added this to the fish-future milestone Oct 15, 2014

@leeola

This comment has been minimized.

Copy link

leeola commented Nov 15, 2014

👍 for the string matching

@faho

This comment has been minimized.

Copy link
Member

faho commented Aug 20, 2015

Everything here that's not concat-and-assign is in #156, so I'm renaming this.

@faho faho changed the title Improve string interaction A simple way to concat-and-assign (like `foo+=bar`) Aug 20, 2015

@faho faho referenced this issue Nov 4, 2016

Closed

set --append? #3485

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented Nov 6, 2016

Just to make it explicit, I'm requesting set -a/--append . :) e.g.

$ set food chicken
$ set -a food soup
$ for f in $food
      echo food: $f
  end
food: chicken
food: soup
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Mar 15, 2017

We're obviously never going to implement foo+=bar. Whether set --append foo bar (or set -a foo bar) is useful enough to justify the fact it is just syntactic sugar for set foo $foo bar is debatable. Also, what about prepending? Do we really need a set -p foo bar that is equivalent to set foo bar $foo? If not doesn't that argue for not adding an append flag?

I'm a big fan of the Zen of Python maxim that

There should be one-- and preferably only one --obvious way to do it.

@krader1961 krader1961 added the RFC label Mar 15, 2017

@krader1961 krader1961 self-assigned this Mar 15, 2017

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Mar 15, 2017

I intend to close this as will-not-implement in the near future unless someone can provide a better argument than is already present in this issue as to why we need --append and --prepend flags to the set command.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Mar 18, 2017

Also, see issue #436 which proposes adding options for appending and prepending to variables.

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented Mar 29, 2017

There should be one-- and preferably only one --obvious way to do it.

I generally agree with that, however a perhaps more important and universal maxim is DRY (don't repeat yourself), and it seems like the set foo $foo bar pattern violates that. I also find that the repetition in close proximity requires more effort to mentally parse compared to set -a foo bar, which I can tell at a glance does an append. And frankly, compared to Bash's foo+=(bar), Fish is just plain tedious: set foo $foo bar. Of course, there's no accounting for taste. ;)

You have a good point about a prepend option. It would seem inconsistent to have one and not the other. I feel like append is a more commonly needed operation, and it would seem prudent to not use up single-letter switches too early, so I would be content with having just the append operation for now, and waiting to see if anyone actually requests prepend.

@krader1961 krader1961 removed their assignment May 3, 2017

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 3, 2017

I am still unconvinced that set --append and set --prepend are useful enough to implement. How is typing set foo $foo bar any more difficult to type, or less clear, than set -a foo bar? Similarly, set foo bar $foo versus set foo -p bar?

The need for an explicit append syntax in bash is primarily due to its adherence to POSIX 1003.1. That affects how variables are interpolated, and split again, on $IFS, into the command. A problem which is not applicable to fish.

Appending and prepending to lists is a common feature of other languages (e.g., C++ and Python). Providing syntactical sugar for doing so in fish might be useful. But I'm not convinced it is useful enough to implement. Nonetheless, despite my previous comment I'm not going to close this "will-not-implement" because I think it deserves more discussion.

@ElijahLynn

This comment has been minimized.

Copy link

ElijahLynn commented May 3, 2017

+1 for set --append and set --prepend, as the current method is quite confusing to me. As a newcomer the current method was kinda confusing and I still find myself looking at the docs every time I am going to append/prepend.

@faho

This comment has been minimized.

Copy link
Member

faho commented May 3, 2017

One thing I've noticed here is that the metasyntactic variables ("foo" and "bar") might be obscuring the issue a bit since they are short. So I submit to you what might be the longest variable name in fish: "___fish_git_prompt_color_flags". Note that it is 30 characters long, so with a common line length limit of 80, you'd have 15 characters to work with on one line (30 * 2 + "set" + 2 spaces between), less with indentation.

 set ___fish_git_prompt_color_flags a $___fish_git_prompt_color_flags something

Now, can you see what I've added to the variable, at a glance? Did you see that I prepended the "a"? Also notice that that is 78 characters, so with our 4-space indentation it could only appear in the global block without going over 80 characters.

With set -a and set -p, this becomes:

set --append ___fish_git_prompt_color_flags something
set --prepend ___fish_git_prompt_color_flags a

Is that more readable, or less?

Granted, this is the worst case and we don't try to make code as short as possible (otherwise we'd have fifty glob qualifiers and curly braces instead of "begin/end"), but we do try to go for readability and in this case I'm having a needle-in-a-haystack experience.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 4, 2017

Now, can you see what I've added to the variable, at a glance?

Yes, I can. Precisely because we don't have flags to append or prepend. I simply looked past the var name being set and saw the first value was a, followed by whatever values were held by the $___fish_git_prompt_color_flags var, then the value something. Your set command could be replaced by echo or any other command. To which your line length and readability concerns are equally applicable. How does that justify making it possible to do this simple operation in more than one way using set?

The main problem I have with this proposal is it doesn't materially make the operation simpler or clearer. Nor do I find arguments about commonly used terminal widths compelling. Especially since the script that defines the var you used in your example, @faho, already has quite a few extremely long lines. Too, few vars have names that long and those that do should arguably be shortened. And in most cases replacing an explicit $var_name with --append saves exactly one character. Obviously that last statement is a bit tongue in cheek since it is a contrived example and we would presumably create -a and -p short flag aliases to the long flags.

Lastly, the issue has been open three years and the implementation is trivial. If someone felt even moderately strongly about this it would be implemented by now.

The one argument that might convince me to implement this would be that it is a significantly faster, more efficient, means of prepending/appending values to vars. I have no doubt that would be true for pathological cases such as a var with a thousand values averaging a thousand chars each. The question is whether it is likely to be measurable, let alone noticeable, with typical fish scripts.

We're probably getting close to the point where the time spent discussing this proposed enhancement is equal to the time required to implement it. What I love about the Python language is the The Zen of Python. Which states "There should be one-- and preferably only one --obvious way to do it." We already have an obvious way to safely prepend and append to variables. I am unconvinced we need another way to do so.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 4, 2017

Also this example

set --prepend ___fish_git_prompt_color_flags a

is confusing. Is a being prepended to ___fish_git_prompt_color_flags or vice-versa? Why isn't it set --prepend=a ___fish_git_prompt_color_flags or set --prepend a ___fish_git_prompt_color_flags? That is rhetorical since since you and I share a common understanding of how this would work. But it is not obvious to me this would be obvious to anyone else. In object oriented languages like Python and C++ this ambiguity doesn't exist since it would be written something like ___fish_git_prompt_color_flags.prepend("a"). But that syntax isn't available to fish.

@ElijahLynn

This comment has been minimized.

Copy link

ElijahLynn commented May 4, 2017

We already have an obvious way to safely prepend and append to variables.

I would like to challenge the use of the word 'obvious' here. I don't find it obvious at all. Do you remember if it was obvious to you the first time? Is it possible you have just done it enough that now it is comfortable to you?

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 5, 2017

Yes, @ElijahLynn, it was obvious to me the first time I needed to append/prepend an element to a var. Because it is consistent with these examples:

echo a $foo b
env EVAR="a:$EVAR:b" a_command arg1

Those and the set case are all simply examples of concatenating new values to the value(s) already assigned to a var. Notice too this statement in one of the early comments:

That is not "concat_and_assign" though, so that gives unwieldy results for non-trivial variables and strings. I'd love to have a set -a foo "bar" to do exactly this (on a string to concat or an array to append a new item).

Except that, assuming $foo contained a single value, this would not append "bar" to the the string -- it would add a second element to the array. If we implement a --append flag there will be people who expect it to append the string to the existing value of a var with a single element. And they will be unpleasantly surprised when it doesn't do so.

I was simply ambivalent about implementing this feature. But given what I just wrote I am now opposed to implementing it.

@faho

This comment has been minimized.

Copy link
Member

faho commented May 5, 2017

If we implement a --append flag there will be people who expect it to append the string to the existing value of a var with a single element. And they will be unpleasantly surprised when it doesn't do so.

That's actually a really good point.

I was simply ambivalent about implementing this feature. But given what I just wrote I am now opposed to implementing it.

I'm convinced as well.

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented May 5, 2017

I respectfully disagree.

  • Using arrays is much easier in Fish than in Bash (given how metacharacter-heavy Bash array syntax is), so users are more inclined to take advantage of arrays in Fish scripts. Therefore it makes sense to support this by making common array operations easier and clearer.
  • String concatenation is already obvious through string interpolation, and Fish even has a dedicated string command now, so it seems perfectly reasonable to me for --append to be an array-specific option.
  • The example of the long variable names in @faho's comment (#1326 (comment)) really drives this point home. Having --append there would be a major improvement.
  • I'm not volunteering anyone else's time, of course. I'm not in a position to help with implementation right now, but I would appreciate leaving this issue open so it can be worked on in the future. Maybe it would be a good candidate for a "help wanted" tag.
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 5, 2017

So you're arguing that we implement a feature that is certain to confuse a non-negligible percentage of users, especially those who recently switched from bash, and doesn't actually implement a new capability just to avoid having to repeat a long variable name? And speaking of confusing behavior here's bash:

bash-4.4$ x=(a b)
bash-4.4$ echo ${x[0]}
a
bash-4.4$ echo ${x[1]}
b
bash-4.4$ echo ${x[2]}

bash-4.4$ echo ${#x}
1
bash-4.4$ echo ${#x[*]}
2
bash-4.4$ x+=c
bash-4.4$ echo ${#x[*]}
2
bash-4.4$ echo ${x[0]}
ac
bash-4.4$ echo ${x[1]}
b
bash-4.4$ echo ${x[2]}

The longer I spend using fish the more perplexed I am why people are so enamored of the steaming mess that is bash/zsh and argue that fish should be more like those shells.

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented May 5, 2017

So you're arguing that we implement a feature that is certain to confuse a non-negligible percentage of users,

I disagree that it would be certain to confuse many people, but who's to say. :)

especially those who recently switched from bash

Fish is very different from Bash, and there is no similar functionality in Bash (i.e. nothing like var--append=val) so I don't think there is additional risk of confusion related to this.

and doesn't actually implement a new capability just to avoid having to repeat a long variable name?

Personally, I appreciate the DRY principle, and I think the example faho gave earlier is a good one (despite his changing his mind since then). One of the purposes of Fish is to provide a syntax with greater clarity, and while the set foo $foo bar syntax is straightforward, the repetition in close proximity is error-prone, and becomes much worse with long variable names like in faho's example.

For those reasons, I think it would be a worthy addition to Fish.

And speaking of confusing behavior here's bash: ... The longer I spend using fish the more perplexed I am why people are so enamored of the steaming mess that is bash/zsh and argue that fish should be more like those shells.

Yes, I agree, Bash's array syntax is absolutely awful. One of the things I like about Fish is that working with arrays is much easier. However, I think that appending to arrays is actually more pleasant in Bash than in Fish. I think adding set --append would remedy that.

My two cents. Cheers.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 5, 2017

there is no similar functionality in Bash (i.e. nothing like var--append=val)

There is:

bash-4.4$ var=(a b)
bash-4.4$ var+=(c)
bash-4.4$ echo ${#var[*]}
3

This issue is arguing for adding the equivalent of that second statement only in a fishy way. The problem as I pointed out with my previous example is that the fact such syntax is required confuses people. If you don't include those parens you end up concatenating to the first element of the array. Adding a set --append will similarly confuse some percentage of fish users. It may be that the benefits of not repeating the var name is greater than the number of questions we'll receive by people expecting it to append to the string (assuming the var has a single element). If there were other benefits from implementing this I'd be for it. But if DRY is the only one we probably shouldn't implement this. Because while DRY (don't repeat yourself) is a good design goal so is the Python maxim that there should ideally be just one obvious way to do something.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented May 6, 2017

@alphapapa, can you provide some justification for your statement:

I think that appending to arrays is actually more pleasant in Bash than in Fish.

My experience is that appending to vars which or may not be arrays in bash/zsh is less pleasant than fish.

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented May 7, 2017

What I mean is, I prefer this:

var+=(element)

over this:

set var $var element

...because the +=() syntax is unambiguously appending to an array, and there is no repetition of the variable name. In the Fish style, the variable name effectively becomes part of the syntax for appending to an array; and since the variable name can be anything, this, in a sense, creates an inconsistent syntax. It's also more error prone, because a one-character misspelling of a variable name might go unnoticed, especially in a long name; and because it is too easy to type, e.g. set $var var element and overlook it.

So a syntax like:

set -a var element

unambiguously appends to an array, and avoids repetition of the variable name, making for consistent syntax.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 4, 2017

I've decided this is actually worthwhile. Primarily because variable expansion is itself moderately expensive. So from the perspective of making fish script fast it would be better to write set -a var another_element rather than set var $var another_element. This also dovetails nicely with the other change I intend to implement to internally store variables as vectors/lists rather than a flat string that has to be converted to a list when that is needed (see issue #4200).

@krader1961 krader1961 modified the milestones: fish-3.0, fish-future Aug 4, 2017

@krader1961 krader1961 removed the RFC label Aug 4, 2017

@krader1961 krader1961 self-assigned this Aug 4, 2017

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 4, 2017

Upon further reflection I am less worried about people being confused regarding the order of the arguments. The proposed syntax is consistent with the existing assignment syntax where the var name without a leading dollar-sign appears first. So I think my earlier concerns, while valid, were overly pessimistic.

krader1961 added a commit to krader1961/fish-shell that referenced this issue Aug 5, 2017

krader1961 added a commit that referenced this issue Aug 5, 2017

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 5, 2017

Merged to major branch for inclusion in fish 3.0.

@krader1961 krader1961 closed this Aug 5, 2017

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented Aug 6, 2017

Thanks for implementing this, Kurtis! Looking forward to using it in my scripts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment