magit-blame break after "eb2079e"

[York Zhao]

Hi,

I noticed that `magit-blame' had broken after commit "eb2079e", not sure if this
happens only in Emacs 24. Have any of you out there experienced similar
problems?

Thank,
York

[Yann Hodique]

Can you be more specific ?
The original commit (eb2079e) had some issues that I fixed in later
commits (actual merge was 559fd1e).
There might be others I didn't spot, though.

Thanks,

Yann.

--
Answers are a perilous grip on the universe. They can appear sensible yet
explain nothing.

-- The Zensunni Whip

[York Zhao]

> Can you be more specific ?

Sure. When I run "M-x magit-blame-mode", I got this error:

magit-blame-split-time: Wrong type argument: integerp, 1314986328.0

Looks like in this line the `lsh' cannot be used on floating point number?

(list (lsh unixtime -16) (logand unixtime #xFFFF)))

I had tried using `round' on the floating point number "unixtime" but got a
"range error" on `round':

(list (lsh (round unixtime) -16) (logand unixtime #xFFFF)))

Thanks,
York

[Yann Hodique]

>>>>> "York" == York Zhao <gtdpl...@gmail.com> writes:

Interesting, it looks like your git version outputs a float when mine
outputs an int. What's your git version ?

I guess `truncate' would be a better choice for making an int out of it,
but I'm not sure what that range error is really about. Hopefully with
your git version I can reproduce and fix.

Thanks,

Yann.

--
One uses power by grasping it lightly. To grasp with too much force is to be
taken over by power, thus becoming its victim.

-- Bene Gesserit Axiom

[York Zhao]

> I guess `truncate' would be a better choice for making an int out of it, but
> I'm not sure what that range error is really about.

I just tried the `truncate' like this:

(defun magit-blame-split-time (unixtime)
"Split UNIXTIME into (HIGH LOW) format expected by Emacs's time functions."
- (list (lsh unixtime -16) (logand unixtime #xFFFF)))
+ (list (lsh (truncate unixtime) -16) (logand unixtime #xFFFF)))

But the result was similar:

> lsh: Arithmetic range error: "truncate", 1335633032.0

But thanks for suggesting the `truncate' function.

> Interesting, it looks like your git version outputs a float when mine outputs
> an int. What's your git version ?

I'm using msysGit v1.7.9, here's the output from "git version":

> git version 1.7.9.GIT

> Hopefully with your git version I can reproduce and fix.

Thanks a lot for the help.

York

[York Zhao]

> But the result was similar:
>
>> lsh: Arithmetic range error: "truncate", 1335633032.0
>

Just noticed that there is new commit trying to address this issue. Pulled and
tested, still getting the same problem:

magit-blame-parse: Arithmetic range error: "truncate", 1329521119.0

I'm not sure but remember reading from somewhere that the biggest integer in
Emacs is not 32bits. If that is true, the number "1329521119" here is
0x4F3EE1DF, which is 31 bits, could be out of the range already. Any idea?


York

[York Zhao]

> magit-blame-parse: Arithmetic range error: "truncate", 1329521119.0
>
> I'm not sure but remember reading from somewhere that the biggest integer in
> Emacs is not 32bits. If that is true, the number "1329521119" here is
> 0x4F3EE1DF, which is 31 bits, could be out of the range already. Any idea?

OK, I have confirmed that the integer number in Emacs is 30 bits, which is the
reason of these problems. I have fixed the issue and sent a pull request. Here
is a description of the algorithm:

The UNIX TIME is a 32 bit integer, however, since integer number in Emacs is
only 30 bits, the UNIX TIME has to be represented by a floating point number.
The algorithm is to first divide the time (floating point number) by 4, which is
equivalent to right shifting 2 bits to make a 30 bits integer number (discarding
the lowest 2 bits). Since this number is the higher 30 bits of the time, by
right shifting 14 bits we get the HIGH (16 bits) part. The original floating
point time mod 4 we get the lowest 2 bits of the time. Take the 30 bits number
calculated in the earlier steps, wipe out the higher 16 bits and we get the
higher 14 bits of the LOW part, left shifting 2 bits and "inclusive or" the
lowest 2 bits we get the final LOW part, therefor, converted the floating point
UNIX TIME to the (HIGH LOW) format representation.

York

[York Zhao]

Hi,

> I have fixed the issue and sent a pull request.

One minute after I sent the pull request, I did a "commit --amend", "push
--force" to my fork at Github, and tried to send the pull request again but
Github said I already had a pull request. From the pull request in the original
repository I can see my new "correct" commit, therefor is there anything I still
need to do here in order for the maintainer to successfully pull my commit? Do I
need to cancel the previous pull request and re-send the new one?

Thanks,
York

[York Zhao]

> One minute after I sent the pull request, I did a "commit --amend", "push
> --force" to my fork at Github, and tried to send the pull request again but
> Github said I already had a pull request. From the pull request in the original
> repository I can see my new "correct" commit, therefor is there anything I still
> need to do here in order for the maintainer to successfully pull my commit? Do I
> need to cancel the previous pull request and re-send the new one?

The commit I wanted to be pulled is: e82fb17


Thanks,
York