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

rc: implement 9atoms `sep{command} syntax #451

Closed
wants to merge 1 commit into from

Conversation

oridb
Copy link
Contributor

@oridb oridb commented Oct 7, 2020

After a bunch of experience using this
syntax in scripts, I want it available
everywhere.

It makes a good deal of code far simpler
to write. For example,

    nl='
    '

    for(f in `$nl{find . -type f})
            munge $f

is more explicit about what is split than
setting and unsetting $ifs, and doesn't
cause unexpected changes to $ifs for rc
functions or subshells that are run inside
the `{} section.

After a bunch of experience using this, I
find that it  makes a good deal of code
far simpler to write. For example,

	nl='
	'

	for(f in `$nl{find . -type f})
		munge $f

ends up quite a bit more pleasant to use
for handling file output line by line,
since it doesn't cause scoping issues
for rc functions, and is more explicit
about what is being split on.
@rsc
Copy link
Contributor

rsc commented Oct 14, 2020

Sorry, but plan9port is only aiming for compatibility with the original Plan 9, not the many forks.

@rsc rsc closed this Oct 14, 2020
@oridb
Copy link
Contributor Author

oridb commented Oct 14, 2020

I wasn't proposing it for compatibility, but for utility. The provenance is there for credit.

Looks like I'll continue running a patched fork. It's a small change to keep updated, and more than carries its weight for me -- though it'll make it more difficult to share scripts.

@ghost
Copy link

ghost commented Oct 14, 2020

I don't mind if this gets merged or not, but, to play devil's advocate, I'd say that the rc parser change that enabled use of unquoted equal signs also wasn't made for compatibility with original Plan 9, no?

@rsc
Copy link
Contributor

rsc commented Oct 15, 2020

It's true that the parser change for = signs was a divergence from Plan 9 rc.
Those are rare and need careful evaluation justification.
In that case, life on Unix means having tons of command lines that use = signs.

This PR is more a whole new feature than a bug fix, and in general I try very hard not to add those.
The justification in this change seemed to be "9atom did it", which doesn't clear the bar for me.

@ghost
Copy link

ghost commented Oct 15, 2020

Ah, I see that now, and it makes perfect sense. Can't argue with that! Thanks for explaining.

@oridb
Copy link
Contributor Author

oridb commented Oct 19, 2020

The justification in this change seemed to be "9atom did it", which doesn't clear the bar for me.

That's my mistake for not going into more detail. I hinted at it, in the description, but to spell it out more explicitly: correct ifs handling means that functions need to be paranoid. You need to wrap functions with code to save and restore ifs if there's a chance that they can be called from a \{}` block wih $ifs changed:

 fn foo {
     oldifs=$ifs
     ifs=' \t'
     do stuff
     $ifs=$oldifs
}

 oldifs=$ifs
 ifs=';'
 for(split in `{foo | semicolon-separated}) use $split
 ifs=$oldifs

Very few functions do it -- and it's clunky enough that it seems worth keeping it that way. $ifs-paranoia boilerplate is ugly. Because this feature removes one of the biggest reasons for manipulating ifs, that dance would usually get replaced with:

 fn foo { do stuff }
 for(split in `';'{foo | semicolon-separated}) use $split

In addition, because it makes splitting more painless, it becomes more common to split filenames on '\n' appropriately, which makes it more likely people will write scripts that work with spaces in file names.

Anyways, if features aren't wanted, that's fine -- I just wanted to make sure that I'd actually described my reasoning.

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.

None yet

2 participants