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

Format #747

Merged
merged 2 commits into from
May 26, 2022
Merged

Format #747

merged 2 commits into from
May 26, 2022

Conversation

nojaf
Copy link
Collaborator

@nojaf nojaf commented May 26, 2022

Hi @dsyme, fantomas-tool was renamed to fantomas so you didn't bump to the latest version.

@dsyme
Copy link
Contributor

dsyme commented May 26, 2022

Hi @dsyme, fantomas-tool was renamed to fantomas so you didn't bump to the latest version.

I see!!!! No wonder I hadn't seen the latest versions :)

@dsyme dsyme merged commit f389c0d into fsprojects:main May 26, 2022
@dsyme
Copy link
Contributor

dsyme commented May 26, 2022

Thank you!!

@nhirschey
Copy link
Collaborator

nhirschey commented Jun 6, 2022

Were the deletions of // FSX and // IPYNB from #endif // FSX conditionals intended? If I leave off // FSX then the #endif is left in the output .fsx file.

For example:

(***condition:fsx***)
#if FSX
let a = 4
#endif

results in an output .fsx with:

let a = 4
#endif

Is this from fantomas?

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 7, 2022

Fantomas does appear not to restore the comment after a hash directive.
Sample.

@dsyme
Copy link
Contributor

dsyme commented Jun 7, 2022

They weren't intended. @nojaf Did you create a tracking issue for this in fantomas?

@dsyme
Copy link
Contributor

dsyme commented Jun 7, 2022

Fixed for this repo via a .fantomasignore entry

#751

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 7, 2022

@dsyme, no I didn't. I'm a bit confused here, is this relevant or not?
If there is meaning to the comment after the hash directive, if can't help but feel that this solution should be reviewed instead.
Fixing this in Fantomas will introduce a messy solution. You want to assign a trivia to a trivia here.
This little manoeuvre won't be pretty and probably introduce a performance hit for everybody.
I'd like to know more before trying to address this on Fantomas' side.
This feels very unorthodox, to be frank.

@dsyme
Copy link
Contributor

dsyme commented Jun 7, 2022

Hmmm, I suspect it's quite common to have clarifying comments in this position, I know I do it all the time to clarify what the #endif is closing. We should probably open a fantomas issue and discuss there even if you decide not to fix, so there's a record of it.

In this case yes, there's a meaning of sorts for the somewhat hacky pre-processing of the scripts (that however could likely be implemented in other ways)

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 8, 2022

This will be painful if we need to check this in larger files: example.

Every comment after source code that is not yet assigned should be checked against hash directives.
This feels like it should be an IDE hint feature instead.
I'm open to discussing this further on Fantomas' side.

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.

3 participants