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

Don't remove "in" from "let ... in ..." binding. #342

Merged
merged 8 commits into from
Oct 30, 2018

Conversation

jindraivanek
Copy link
Contributor

Fix #340.

Result of pair programming with @nojaf on FableConf :)

@nojaf
Copy link
Contributor

nojaf commented Oct 30, 2018

Could you please merge in master. Some tests are failing but I'm not sure that my changes caused this.
Merging in master might resolve this.

@nojaf
Copy link
Contributor

nojaf commented Oct 30, 2018

Thanks for merging, I'm a bit puzzled why this test is suddenly failing.

It doesn't work with 2.9 and I don't see why these changes would change this behavior.

@@ -619,7 +619,11 @@ and genExpr astContext = function

| TypeApp(e, ts) -> genExpr astContext e -- "<" +> col sepComma ts (genType astContext false) -- ">"
| LetOrUses(bs, e) ->
atCurrentColumn (genLetOrUseList astContext bs +> sepNln +> genExpr astContext e)
let isInSameLine =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also check if the 'in' word was used in the original source code?

@nojaf nojaf merged commit 60d0ac5 into fsprojects:master Oct 30, 2018
nojaf added a commit that referenced this pull request Oct 31, 2018
* Allow easy build/format/build cycle of external projects (#233) (#254)

* Initial implementation of testing external projects (git clone, build, format, build again). For now tests with the Argu project which fails ATM

* Improve building of external projects for regression testing.

Add configuration function that allows platform specific configuration of external projects build exectuable and arguments
TestExternalProjects is now dependent on Build only for faster testing
Platform specific selection of Fanotmas executable from previous build step
Shallow cloning of external repos for faster clones; switched to always cloning fresh repo instead of resetting

* Fix 452 path

* Attempt to add build stages to enable long running external tests on certain conditions in travisCI

* Set sudo to true which will AFAIK disable container based builds

* Switch back to container based builds, try Argu as an example external project on CI

* Reverting sudo to true again since container builds take much more time in startup

* Improve build script process name/args handling, fix paths for nix dotnet core testing

* Cleanup of accidental changes vs master

* More cleanup

* Revert accidental whitespace changes in test file

* Add testing of external projects only when on a branch named build-external-projects

* Final cleanup and comments

* Dummy commit to trigger rebuilds after github issues

* Don't remove "in" from "let ... in ..." binding. (#342)

* test

* Use "let .. in .." if pattern and expr are at the same line.

* Added fix for comment after in

* Revert removed lines from fsproj

* Merge fix

* Fixed failing tests

* add external projects, TestExternalProjectsFailing target, allow to specify project (#338)
@jindraivanek jindraivanek deleted the issue-340 branch April 9, 2019 08:13
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.

None yet

2 participants