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

Inline mode not supported by NetBSD's sed #168

Open
DK1MI opened this issue Sep 8, 2015 · 7 comments · May be fixed by #421
Open

Inline mode not supported by NetBSD's sed #168

DK1MI opened this issue Sep 8, 2015 · 7 comments · May be fixed by #421

Comments

@DK1MI
Copy link

DK1MI commented Sep 8, 2015

On NetBSD (and probably other BSDs) sed is not supporting the inline mode todo.txt is using (sed -i).
A quick workaround is to replace "sed" with "gsed" in todo.sh. Please either let the user define which sed version to use, e.g. in .todo/config or don't use sed's inline mode.

@iamleot
Copy link

iamleot commented May 27, 2018

Hello @exitnode !
We use the same workaround for the time/todotxt pkgsrc package (and the following sed(1) incantation does the trick!: sed -E -e 's,([(| ])sed ,\1gsed ,g)

What about introducing a ${SED} variable and replace all sed invocations with that?

(If that sounds reasonable please let me know and I'll try to write a possible patch about that!)

Thank you!

@inkarkat
Copy link
Member

What about introducing a ${SED} variable

You don't need that.

let the user define which sed version to use, e.g. in .todo/config

You can define a sed function in the config file and export it; todo.sh and any add-ons launched by it will then use that one:

export originalSed=$(which sed)
sed()
{
    # Assumption: The in-place option is passed as the first argument.
    if [ "$1" = '-i.bak' ]; then
        shift

        # Assumption: Only a single file is processed, and it is passed as the
        # last argument.
        file=${!#}

        tmpFile=/tmp/todo.sh-sed.$$
        "$originalSed" "$@" > "$tmpFile" && mv "$tmpFile" "$file"
    else
        "$originalSed" "$@"
    fi
}
export -f sed

This one emulates the sed -i without requiring gsed. If you have the latter, the override becomes even simpler:

sed()
{
    # Use gsed because it understands the -i in-place option.
    gsed "$@"
}
export -f sed

@karbassi
Copy link
Member

@inkarkat should we include this in the documentation somewhere?

@iamleot
Copy link

iamleot commented May 27, 2018 via email

@inkarkat
Copy link
Member

@karbassi I think this is only the first or maybe second time this came up. BSD isn't a terribly common platform, and most users are quite knowledgeable (and quite used to dealing with these compatibility issues). I'd wait until we see another such question, and now this should be more discoverable through this issue.

@iamleot Are you really using BSD as your sole platform and intend to do todo.sh dev work? I once used FreeBSD to verify the compatibility with Mac OS (don't have any Apple hardware, and Gina always tested on Mac) - fortunately, Mac OS has suitable sed and awk versions. The (huge) downside of @SED@ is that one is forced to run make; the script won't work without it any longer. Second: If you automate this, do you assume gsed is available, or even implement both gsed and the other workaround?! To me, this adds a lot of complexity without much benefit.

@iamleot
Copy link

iamleot commented May 27, 2018 via email

@hyperupcall
Copy link
Contributor

I created a linked PR that solves this, but it does not export the function for use by add-ons. This is because the new sed function's carries assumptions that may not remain true in all add-on code. Therefore, the scope of this fix includes code within this file's todo.sh only, which is probably good enough anyways.

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

Successfully merging a pull request may close this issue.

5 participants