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

flip-flop ! (template string formatting is unstable) #13

Closed
vicb opened this issue Jun 2, 2015 · 8 comments
Closed

flip-flop ! (template string formatting is unstable) #13

vicb opened this issue Jun 2, 2015 · 8 comments

Comments

@vicb
Copy link
Contributor

vicb commented Jun 2, 2015

flip:

  compile(template: ViewDefinition): Promise<ProtoViewDto> {
    var tplPromise = this._templateLoader.load(template);
    return PromiseWrapper.then(
        tplPromise, (el) => this._compileTemplate(template, el, ProtoViewDto.COMPONENT_VIEW_TYPE),
        (e) => {
          throw new BaseException(`Failed to load the template for "${template.componentId}": ${e}`);
        });
  }

flop:

  compile(template: ViewDefinition): Promise<ProtoViewDto> {
    var tplPromise = this._templateLoader.load(template);
    return PromiseWrapper.then(
        tplPromise, (el) => this._compileTemplate(template, el, ProtoViewDto.COMPONENT_VIEW_TYPE),
        (e) => {
          throw new BaseException(
              `Failed to load the template for "${template.componentId}": ${e}`);
        });
  }

... infinite loop !

as it changes in every single run, the format check also fails on every single run !

/cc @mprobst

@mprobst
Copy link
Contributor

mprobst commented Jun 2, 2015

It's miscounting the length of the template string. I'll look into it. For now, the workaround is:

class X {
  compile(template: ViewDefinition): Promise<ProtoViewDto> {
    var tplPromise = this._templateLoader.load(template);
    return PromiseWrapper.then(
        tplPromise, (el) => this._compileTemplate(template, el, ProtoViewDto.COMPONENT_VIEW_TYPE),
        (e) => {
          var msg = `Failed to load the template for "${template.componentId}": ${e}`;
          throw new BaseException(msg);
        });
  }
}

@vicb
Copy link
Contributor Author

vicb commented Jun 2, 2015

I have "fixed" this by adding a space before the ':'

@vicb
Copy link
Contributor Author

vicb commented Jun 2, 2015

Thanks for looking into this.

@vicb vicb changed the title flip-flop ! flip-flop ! (template string formatting is unstable) Jul 1, 2015
@alexeagle
Copy link
Contributor

to consider: we could improve the error reporting here by double-formatting inside gulp-clang-format, to ensure that the proposed formatting change is stable. If we find that the second formatting pass produces a different answer, we could silence the error for that file.
I believe we've seen a few different bugs manifest in this way, so perhaps this is not the last one either?

@mprobst
Copy link
Contributor

mprobst commented Jul 2, 2015

@alexeagle I'll see if I can do something about this, this happens more often than I thought. I don't think changing gulp-clang-format to run twice is a good idea; this is just a bug in clang-format that should be fixed.

@vicb
Copy link
Contributor Author

vicb commented Jul 2, 2015

this is just a bug in clang-format that should be fixed.

👍

@mprobst
Copy link
Contributor

mprobst commented Jul 2, 2015

I've just released v1.0.26 that should fix these issues for template literals.

@mprobst mprobst closed this as completed Jul 2, 2015
@vicb
Copy link
Contributor Author

vicb commented Jul 2, 2015

Thanks !

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

No branches or pull requests

3 participants