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

Support for spaces in marbles strings #2842

Closed
rkrisztian opened this issue Sep 21, 2017 · 6 comments
Closed

Support for spaces in marbles strings #2842

rkrisztian opened this issue Sep 21, 2017 · 6 comments

Comments

@rkrisztian
Copy link

rkrisztian commented Sep 21, 2017

The following example can be read in Anatomy of a Test:

var e1 = hot('----a--^--b-------c--|');
var e2 = hot(  '---d-^--e---------f-----|');
var expected =      '---(be)----c-f-----|';

expectObservable(e1.merge(e2)).toBe(expected);

However, this kind of formatting is not auto-formatter friendly, and I would not follow incompatible practices. And if I recall, the Clean Code book by Robert C. Martin also advises against columned alignment.

What would work however if you let those marbles strings contain spaces. Having to trim manually is an overhead however that I would not do myself, to keep things easily readable.

Edit: I know that we already started using columned alignment in the strings. But by exceeding the boundaries of strings for columned alignment, we make this problem even worse. So all I'm saying is that we keep this special formatting inside the strings where no auto-formatter cares what we do in it.

@benlesh
Copy link
Member

benlesh commented Sep 22, 2017

AFAIK, this is supported now. I haven't tried it in a while though.

@rkrisztian
Copy link
Author

While spaces are allowed, they influence the frame count, which is bad in my case because I need it only for alignment.

@kwonoj
Copy link
Member

kwonoj commented Sep 25, 2017

if you're ok with 3rd party module, peek https:/kwonoj/rx-sandbox . (disclaimer: I wrote it) which support whitespace align in marbles.

@cartant
Copy link
Collaborator

cartant commented Jan 19, 2020

Closing this because spaces have no meaning when the TestScheduler is used in run mode.

@cartant cartant closed this as completed Jan 19, 2020
@rkrisztian
Copy link
Author

rkrisztian commented Jan 19, 2020

@cartant, I was a bit confused at this reasoning first, just to clarify, this issue was opened a long time ago, I can see the user guide updated now with the following:

' ' whitespace: horizontal whitespace is ignored, and can be used to help vertically align multiple marble diagrams.

As well as:

testScheduler.run(({ hot, expectObservable }) => {
  const source = hot('--a--a--a--a--a--a--a--');
  const sub1 = '      --^-----------!';
  const sub2 = '      ---------^--------!';
  const expect1 = '   --a--a--a--a--';
  const expect2 = '   -----------a--a--a-';

  expectObservable(source, sub1).toBe(expect1);
  expectObservable(source, sub2).toBe(expect2);
});

That is cool, and thanks.

@cartant
Copy link
Collaborator

cartant commented Jan 19, 2020

Note that it's only ignored when the run method is used. The run method was introduced after this issue was opened and its behaviour is very different from that of the original TestScheduler implementation.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants