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.

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

Minor Seq Optimizations #922

Merged
merged 4 commits into from
Jun 21, 2024
+105 −89
Merged

Minor Seq Optimizations #922

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e74c84
Update Seq.filter + maps into Seq.choose
1eyewonder Jun 13, 2024
82f7cbe
Updated analyzers
1eyewonder Jun 19, 2024
c3cef1b
Updated per review comments
1eyewonder Jun 19, 2024
6b8d5c9
Additional collection optimizations
1eyewonder Jun 20, 2024
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
7 changes: 5 additions & 2 deletions src/Common/StringParsing.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,11 @@ module String =
let removeSpaces (lines: string list) =
let spaces =
lines
|> Seq.filter (String.IsNullOrWhiteSpace >> not)
|> Seq.map (fun line -> line |> Seq.takeWhile Char.IsWhiteSpace |> Seq.length)
|> Seq.choose (fun line ->
if not <| String.IsNullOrWhiteSpace line then
1eyewonder marked this conversation as resolved.
Show resolved Hide resolved
line |> Seq.takeWhile Char.IsWhiteSpace |> Seq.length |> Some
else
None)
|> fun xs -> if Seq.isEmpty xs then 0 else Seq.min xs

lines
Expand Down
7 changes: 5 additions & 2 deletions src/FSharp.Formatting.ApiDocs/GenerateModel.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2264,8 +2264,11 @@ module internal SymbolReader =
do
replacedParagraphs
|> Seq.collect collectParagraphIndirectLinks
|> Seq.filter (linkNotDefined doc)
|> Seq.map (getTypeLink ctx)
|> Seq.choose (fun line ->
if linkNotDefined doc line then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please invert the if/else here, I prefer the negative branch to be the if branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated to use a local variable to prevent a double negative. Let me know if you prefer to not use this

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like updating the definition of linkNotDefined to linkDefined might also be an option. It is only used once and could probably ne simplified to a List.exist instead of the map/reduce combo.

getTypeLink ctx line |> Some
else
None)
|> Seq.iter (addLinkToType doc)

doc.With(paragraphs = replacedParagraphs)
Expand Down
13 changes: 9 additions & 4 deletions src/FSharp.Formatting.Common/Templating.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ type FrontMatterFile =
// Allow empty lines in frontmatter
let isBlankLine = String.IsNullOrWhiteSpace line
isBlankLine || line.Contains(":"))
|> Seq.filter (String.IsNullOrWhiteSpace >> not)
|> Seq.map (fun line ->
let parts = line.Split(":")
parts.[0].ToLowerInvariant(), parts.[1])
|> Seq.choose (fun line ->
let parts = line.Split(":") |> Array.toList
1eyewonder marked this conversation as resolved.
Show resolved Hide resolved

if not <| String.IsNullOrWhiteSpace line then
1eyewonder marked this conversation as resolved.
Show resolved Hide resolved
match parts with
| first :: second :: _ -> Some(first.ToLowerInvariant(), second)
| _ -> None
else
None)
|> Map.ofSeq

match
Expand Down
Loading