Test: add set orientation #16
Conversation
src/index.ts
Outdated
| @@ -76,6 +76,13 @@ export async function getOrientation ( | |||
| return info; | |||
| } | |||
|
|
|||
| export async function setOrientation ( | |||
| input: File | Buffer | ArrayBuffer, | |||
| orientation: number, | |||
There was a problem hiding this comment.
It would be better to have this orientation type OrientationCode | IOrientationInfo
There was a problem hiding this comment.
I swapped to OrientationCode for now. I think I'll need a new test for to drive myself into adding IOrientationInfo.
There was a problem hiding this comment.
Sorry I now think IOrientationInfo is a little bit too much. Only OrientationCode would be great.
There was a problem hiding this comment.
I was thinking the same thing
TonyBrobston
force-pushed
the
implement-set-orientation
branch
8 times, most recently
from
73c77b5
to
df3cf0e
Compare
| function getOrientationOffsetAndLittleEndian (view: DataView, segmentOffset: number) { | ||
| const tiffHeaderOffset = segmentOffset + statics.offsets.tiffHeader.fromSegment; | ||
| const littleEndian = isLittleEndian(view, tiffHeaderOffset); | ||
| const ifdPosition = findIfdPosition(view, tiffHeaderOffset, littleEndian); | ||
| const ifdFieldOffset = ifdPosition + statics.ifdFieldCountLength; | ||
| const orientationOffset = findOrientationOffset( | ||
| view, | ||
| ifdFieldOffset, | ||
| littleEndian, | ||
| ); | ||
| return {littleEndian, orientationOffset}; | ||
| } |
There was a problem hiding this comment.
I don't love this. Trying to reduce duplication.
TonyBrobston
force-pushed
the
implement-set-orientation
branch
from
df3cf0e
to
aa6598f
Compare
|
@ginpei I've finished my first round of coding, let me know what changes you would like. |
|
@ginpei Are you able to give me some feedback on these changes? |
src/index.ts
Outdated
| @@ -76,6 +76,13 @@ export async function getOrientation ( | |||
| return info; | |||
| } | |||
|
|
|||
| export async function updateOrientation ( | |||
There was a problem hiding this comment.
Can you delete this updateOrientation() function? Let's have only updateOrientationCode().
The original idea of the difference between getOrientation() and readOrientationCode() is which type they return. The "code" one returns OrientationCode and the other returns IOrientationInfo. So this updateOrientation() should accept "info" if we implement.
I'm sorry I should've told you this before but I needed time to remember.
There was a problem hiding this comment.
This makes sense. No need to be sorry, I was asking for feedback :). I deleted the function as you requested.
|
I added one last request. I will merge and release package soon right after updated. |
TonyBrobston
force-pushed
the
implement-set-orientation
branch
from
aa6598f
to
4e97c99
Compare
|
I just realized I haven't updated the readme at all, is that something you would mind handling? If not, I can give it a try. |
|
Good point. Please go ahead. |
|
@ginpei I updated the documentation. I tried to follow your pattern. |
README.md
Outdated
| ```ts | ||
| import * as exif from '@ginpei/exif-orientation'; | ||
|
|
||
| await exif.updateOrientationCode(fileOrBuffer, 5); |
There was a problem hiding this comment.
Use exif.OrientationCode.deg90Flipped instead please
|
@TonyBrobston Requested one thing. Looks good to me other than that. Do you want to do anything else before merging? I will prepare release note with your name and account. |
|
Done. Everything seems good to me 👍 . |
Here's the beginning of
setOrientationif you're open to implementing it. I could look into the implementation, just figured I'd start with a test.Relates to #15