Skip to content

fix for parsing output path when condition has no spaces #1058

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.

Sign up for GitHub

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

Merged
forki merged 2 commits into from
Sep 12, 2015
Merged

fix for parsing output path when condition has no spaces #1058

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d43d58
fix for parsing output path when condition has no spaces
Sep 11, 2015
9ade29e
fix parseComparison using less naive approach
Sep 11, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Paket.Core/ProjectFile.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ type ProjectFile =
let c = input.[index]
if data.Length = 0 && c = ' '
then parseComparison data input (index + 1)
elif data.Length = 2 && c <> ' '
elif data.Length = 2 && c <> ' ' && c <> '\''

@allykzam allykzam Sep 11, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What you wrote here should work fine for #1057.
If I had thought of conditions without spaces when I wrote the parser, I might have written this line as something like so:

let isCompChar c = c = '<' || c = '>' || c = '!' || c = '='
...
elif data.Length = 2 && isCompChar c

(helper function so the next elif could be simplified as well)
If you want to do that with this PR, let me know how the tests go; otherwise this looks good to me 👍

@ascjones ascjones Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed you're right that is a significantly less naive approach. Pull request amended.

then None
elif c = '<' || c = '>' || c = '!' || c = '='
then parseComparison (data.Append(c)) input (index + 1)
Expand Down
2 changes: 1 addition & 1 deletion tests/Paket.Tests/ProjectFile/OutputSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ let ``should detect target framework for Project2 proj file``() =

[<Test>]
let ``should detect output path for proj file``
([<Values("Project1", "Project2", "Project3")>] project)
([<Values("Project1", "Project2", "Project3", "ProjectWithConditions")>] project)
([<Values("Debug", "Release")>] configuration) =
ProjectFile.Load(sprintf "./ProjectFile/TestData/%s.fsprojtest" project).Value.GetOutputDirectory configuration
|> shouldEqual (System.IO.Path.Combine(@"bin", configuration) |> normalizePath)
Expand Down
4 changes: 2 additions & 2 deletions tests/Paket.Tests/ProjectFile/TestData/ProjectWithConditions.fsprojtest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
<RestorePackages>true</RestorePackages>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
<PropertyGroup Condition=" '$(Configuration)|$(Platform)'=='Debug|AnyCPU' ">
<DebugSymbols>true</DebugSymbols>
<DebugType>full</DebugType>
<Optimize>false</Optimize>
Expand All @@ -31,7 +31,7 @@
<StartArguments>
</StartArguments>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<PropertyGroup Condition=" '$(Configuration)|$(Platform)'=='Release|AnyCPU' ">
<DebugType>pdbonly</DebugType>
<Optimize>true</Optimize>
<Tailcalls>true</Tailcalls>
Expand Down