remove extra whitespace around <- when using ess-insert-assign

[plantarum]

The old version used delete-white-space to remove extra spaces around the assignment operator. For those of us still using this, it would be nice to get that convenience back.

[mmaechler]

Thank you; I agree that the default should remove white space, hence I've merged your PR. OTOH, I'm someone who uses whitespace carefully notably to align lines with similar/identical code such that the similarity is more quickly seen by the (human) code reader. Consequently, sometimes I do want to keep the whitespace which I may have inserted on purpose, previously to use the assign key. Ideally we'd get the option to use C-u (the emacs prefix) triggering a second optional argument to ess-insert-assign which would not remove white space.
Would you like to provide that possibility as well with a second pull request?

[lionel-]

My reservation about merging this PR is that we don't know if this broke existing unit tests (travis no longer works apparently) and the PR doesn't add new tests for the behaviour so we don't know if the feature is working correctly.

@mmaechler I'm not sure it's necessary to add that option because in most situations where we insert assignment there won't be existing spaces already aligned. In most cases we create a bunch of unaligned assignments and then use align-regexp to align them. And if there is an existing alignment, other workflows like rectangle filling or multiple cursors are probably quicker than inserting assignments one by one with an additional C-u keystroke each time. Can you provide a before/after example where optionally turning off space styling is useful?

[plantarum]

Martin Mächler @.***> writes:

Ideally we'd get the option to use C-u (the emacs prefix) triggering a second optional argument to ess-insert-assign which would not remove white space. Would you like to provide that possibility as well with a send pull request?

I can do that, but it may take me a few days to get back to this.

ty

[lionel-]

@plantarum I've added a unit test for your feature in https://github.com/emacs-ess/ESS/commit/df297416.

And I've added the feature to ess-cycle-assign as well (this is now the main function for this task) in https://github.com/emacs-ess/ESS/commit/a5df6aa8.

I can do that, but it may take me a few days to get back to this.

Maybe best finish the discussion to see if this feature is needed? Adding C-u behaviour has a cost, for instance it gets in the way of other features that could be implemented in the future. Speaking of which, there is actually an existing C-u behaviour for ess-insert-assign, so we'd have to see which one is more useful if we were to change it.

[mmaechler]

@lionel- I wouldn't have expected that "outsiders" must accompany code changes with unit tests, because that may raise the barrier too much. Of course thanks for adding the unit test. To my C-u-proposal for keeping previous behavior: Often previous behavior is worth being still available if you change a default. OTOH, this was a new default that one could have regarded an accident, itself not keeping (nor optionally providing) previous behavior. So I'm fine if we keep it as it is now.

[lionel-]

That makes sense. It'd be nice to get a new CI up so that at least existing unit tests can be checked with contributed code.