From 878c2cb95c2f65f90bfe354f479ca1af445dc410 Mon Sep 17 00:00:00 2001 From: Glenn Date: Wed, 12 Jul 2023 11:27:21 +0200 Subject: [PATCH] refactor: improve the Range class --- spec/commit_spec.cr | 3 +- spec/range_spec.cr | 101 +++++++++++++++++++++++++++++++++++-- src/timecost/author.cr | 4 ++ src/timecost/commit.cr | 11 ++-- src/timecost/git_reader.cr | 1 + src/timecost/range.cr | 81 +++++++++++++++-------------- 6 files changed, 151 insertions(+), 50 deletions(-) diff --git a/spec/commit_spec.cr b/spec/commit_spec.cr index 36d6c00..a2dcd61 100644 --- a/spec/commit_spec.cr +++ b/spec/commit_spec.cr @@ -12,9 +12,10 @@ describe TimeCost::Commit do ) commit = TimeCost::Commit.new( - commit_hash: "53c01d0db42ac662ed1aff3799d2a92a04e03908", + commit_hash: Random::Secure.base64(40), datetime: Time.utc, author: author, + message: "First commit" ) commit.should be_a(TimeCost::Commit) end diff --git a/spec/range_spec.cr b/spec/range_spec.cr index 4d946bb..d4b1917 100644 --- a/spec/range_spec.cr +++ b/spec/range_spec.cr @@ -14,30 +14,35 @@ describe TimeCost::Range do email: "jon.snow@example.com" ) commit_base = TimeCost::Commit.new( - commit_hash: Random::Secure.base64(40), + commit_hash: Random::Secure.hex(40), author: author, datetime: Time.utc, + message: "Commit base" ) commit_overlap_before = TimeCost::Commit.new( - commit_hash: Random::Secure.base64(40), + commit_hash: Random::Secure.hex(40), author: author, datetime: Time.utc - overlap_time, + message: "Commit with overlap before" ) commit_overlap_after = TimeCost::Commit.new( - commit_hash: Random::Secure.base64(40), + commit_hash: Random::Secure.hex(40), author: author, datetime: Time.utc + overlap_time, + message: "Commit with overlap after" ) commit_separate_before = TimeCost::Commit.new( - commit_hash: Random::Secure.base64(40), + commit_hash: Random::Secure.hex(40), author: author, datetime: Time.utc - separate_time, + message: "Commit separate before" ) commit_separate_after = TimeCost::Commit.new( - commit_hash: Random::Secure.base64(40), + commit_hash: Random::Secure.hex(40), author: author, datetime: Time.utc + separate_time, + message: "Commit separate after" ) @@ -58,6 +63,16 @@ describe TimeCost::Range do end end + describe ".to_s" do + it "must display something" do + range_base = TimeCost::Range.new( + commit: commit_base + ) + str = range_base.to_s + puts str + end + end + describe ".overlap?" do it "must return true when range overlaps range before" do range_base = TimeCost::Range.new( @@ -115,5 +130,81 @@ describe TimeCost::Range do overlap2.should be_false end end + + describe ".add" do + pending "must add the commit to the range" + + pending "must not duplicate existing commits" + + pending "must change the boundaries" + end + + describe ".merge" do + it "must return a merged range with all commits" do + range_base = TimeCost::Range.new( + commit: commit_base + ) + range_before = TimeCost::Range.new( + commit: commit_overlap_before + ) + range_after = TimeCost::Range.new( + commit: commit_overlap_after + ) + + range_result1 = range_base.merge(range_before) + range_result1.commits.size.should eq(2) + + range_result2 = range_base.merge(range_after) + range_result2.commits.size.should eq(2) + end + + pending "must not include a commit twice" + + it "must return a merged range with correct boundaries" do + range_base = TimeCost::Range.new( + commit: commit_base + ) + range_before = TimeCost::Range.new( + commit: commit_overlap_before + ) + range_after = TimeCost::Range.new( + commit: commit_overlap_after + ) + + range_result1 = range_base.merge(range_before) + range_result1.time_start.should eq(range_before.time_start) + range_result1.time_stop.should eq(range_base.time_stop) + + range_result2 = range_base.merge(range_after) + range_result2.time_start.should eq(range_base.time_start) + range_result2.time_stop.should eq(range_after.time_stop) + end + + it "must fail with error when separate before" do + range_base = TimeCost::Range.new( + commit: commit_base + ) + range_before = TimeCost::Range.new( + commit: commit_separate_before + ) + + expect_raises(TimeCost::Range::MissingOverlapError) do + range_base.merge(range_before) + end + end + + it "must fail with error when separate after" do + range_base = TimeCost::Range.new( + commit: commit_base + ) + range_after = TimeCost::Range.new( + commit: commit_separate_after + ) + + expect_raises(TimeCost::Range::MissingOverlapError) do + range_base.merge(range_after) + end + end + end end diff --git a/src/timecost/author.cr b/src/timecost/author.cr index 805b40e..1727788 100644 --- a/src/timecost/author.cr +++ b/src/timecost/author.cr @@ -12,5 +12,9 @@ module TimeCost (self.email == other_author.email) ) end + + def to_s + "#{self.name} <#{self.email}>" + end end end diff --git a/src/timecost/commit.cr b/src/timecost/commit.cr index c5c4c8c..f449e0b 100644 --- a/src/timecost/commit.cr +++ b/src/timecost/commit.cr @@ -4,13 +4,14 @@ require "./author" module TimeCost class Commit - property author : Author - property commit_hash : String + getter author : Author + getter commit_hash : String - property datetime : Time - property message : String + getter datetime : Time + getter message : String + getter notes : String - def initialize(@commit_hash, @datetime=Time, @author=Author, @message="") + def initialize(@commit_hash, @datetime, @author, @message, @notes="") # @note = nil end end diff --git a/src/timecost/git_reader.cr b/src/timecost/git_reader.cr index 5304e3d..2a6a411 100644 --- a/src/timecost/git_reader.cr +++ b/src/timecost/git_reader.cr @@ -40,6 +40,7 @@ module TimeCost datetime: Time.parse!(json["date"].to_s, "%F %T %z"), author: author, message: json["message"].to_s, + notes: json["notes"].to_s ) end diff --git a/src/timecost/range.cr b/src/timecost/range.cr index ef7a090..ba01733 100644 --- a/src/timecost/range.cr +++ b/src/timecost/range.cr @@ -1,7 +1,10 @@ require "./commit" module TimeCost + class Range + class MissingOverlapError < RuntimeError ; end + property time_start property time_stop property commits : Array(Commit) @@ -24,34 +27,35 @@ module TimeCost @commits.first.try &.author end - def merge(range) - # B -----[----]---- - # A --[----]------ - # = ---[------]---- + def add(commit : Commit) : Range + end - # minimum of both - new_start = ( - if range.time_start < @time_start - range.time_start - else - @time_start - end - ) + def merge!(other_range) : Range + raise MissingOverlapError.new if @time_stop < other_range.time_start + raise MissingOverlapError.new if @time_start > other_range.time_stop - new_end = ( - if range.time_stop >= @time_stop - range.time_stop - else - @time_stop - end - ) + # B ------[----]---- + # A --[----]------- + # = ---[-------]---- + + # boundaries + new_start = [@time_start, other_range.time_start].min + new_end = [@time_stop, other_range.time_stop].max @time_start = new_start @time_stop = new_end - @commits.concat range.commits + @commits.concat other_range.commits + self end - def overlap?(range) + def merge(other_range) : Range + copy = self.dup + copy.commits = self.commits.dup + + copy.merge!(other_range) + end + + def overlap?(range) : Bool result = false # return early result if ranges come from different authors @@ -104,27 +108,26 @@ module TimeCost return result end - def fixed_start - return @time_start + (@epsilon/24.0) + def diff_hours : Float + return ((@time_stop - @time_start) / (60* 60)).to_f end - def diff : Float - return ("%.2f" % ((@time_stop - fixed_start).to_f * 24)).to_f - end - - def to_s(show_authors = true) - val = "(%s)\t%s - %s\n" % [diff, fixed_start, @time_stop] - if show_authors - val += "\tby %s\n" % @commits.first.author + def to_s(show_authors = true) : String + result = String.build do |val| + val << "(%.2f)\t%s - %s\n" % [diff_hours, @time_start, @time_stop] + if show_authors + val << "\tby %s\n" % @commits.first.author.to_s + end + @commits.each do |commit| + lines = commit.message.split(/\n/) + r = lines.map_with_index do |line,i| + x = (i == 0) ? "[#{commit.commit_hash[0..7]}]" : " " * 9 + "\t#{x} #{line}" + end.join("\n") + val << r + "\n" + end end - @commits.each do |commit| - lines = [] of String - lines.concat commit.note.split(/\n/) - r = lines.map { |s| "\t %s" % s }.join "\n" - r[1] = '*' - val += r + "\n" - end - return val + result end end end