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 pretty-printing without inlining #include-d code #1042

Closed
spinkney opened this issue Nov 16, 2021 · 4 comments · Fixed by #1069
Closed

Support pretty-printing without inlining #include-d code #1042

spinkney opened this issue Nov 16, 2021 · 4 comments · Fixed by #1069
Assignees
Labels
feature New feature or request good first issue Good for newcomers

Comments

@spinkney
Copy link
Contributor

When the '.stanfunctions' file has an '#include some.stanfunctions' in a different directory the auto formatter fails. One typically wouldn't include the path because it would be specified in the call to cmdstanr via include_paths = "./path_to_file".

@WardBrian
Copy link
Member

I think the issue is a little subtle. Consider two files:

test1.stanfunctions

// defined in test1.stanfunctions
void foo(){
    return;
}

test2.stanfunctions

#include "test1.stanfunctions"

// defined in test2.stanfunctions
void bar(){
    foo();
}

Now, stanc --auto-format test2.stanfunctions fails, but that is intentional. In order to form an AST, we need the include information. Especially if you want to use --print-canonical, we need to be able to actually type-check that AST.

So, stanc --auto-format test2.stanfunctions --include-paths=/path/to/folder is what you want, but, it outputs this:

// defined in test1.stanfunctions
void foo() {
  return;
}

// defined in test2.stanfunctions
void bar() {
  foo();
}

So I think the real issue here is "support pretty-printing without inlining #include-d code". This should be doable because we keep the fact that it was included around in the location information.

@WardBrian WardBrian added the feature New feature or request label Nov 17, 2021
@WardBrian WardBrian changed the title Auto format of '.stanfunctions' fails when include is not in the directory Support pretty-printing without inlining #include-d code Nov 17, 2021
@WardBrian WardBrian changed the title Support pretty-printing without inlining #include-d code Support pretty-printing without inlining #include-d code Nov 17, 2021
@WardBrian WardBrian added the good first issue Good for newcomers label Nov 17, 2021
@spinkney
Copy link
Contributor Author

So I think the real issue here is "support pretty-printing without inlining #include-d code". This should be doable because we keep the fact that it was included around in the location information.

Yes, this would work

@rok-cesnovar
Copy link
Member

I agree that by default auto-format should not inline #include. I would like to still have a way to inline the includes if requested. Not sure on what the API should be.

The use case is caching models without having to inline includes with some string processing.

@WardBrian
Copy link
Member

I've been thinking about how the interface should be for formatting, canonicalizing, etc. I think all of these things should be optional and part of the canonicalize command, with --auto-format doing the bare minimum.

So I think we'd want something like --auto-format --canonical=includes,deprecations,parenthesis with --print-canonical becoming an alias for essentially --auto-format --canonical=[everything we can do]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants