Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1297083002: Merge to XFA: Cleanup CFX_UnicodeEncoding and remove IFX_FontEncoding. (Closed)

Created:
5 years, 4 months ago by Lei Zhang
Modified:
5 years, 4 months ago
Reviewers:
Tom Sepez, Nico
CC:
pdfium-reviews_googlegroups.com, Nico
Base URL:
https://pdfium.googlesource.com/pdfium.git@xfa
Target Ref:
refs/heads/xfa
Visibility:
Public.

Description

Merge to XFA: Cleanup CFX_UnicodeEncoding and remove IFX_FontEncoding. Similarly, Clean up CFX_UnicodeEncodingEx and remove IFX_FontEncodingEx. R=tsepez@chromium.org Review URL: https://codereview.chromium.org/1296753003 . (cherry picked from commit 52a48aadc19b2dee8abeb702269bb168eb6b6999) Committed: https://pdfium.googlesource.com/pdfium/+/1b0023986bc22ce362097b25deb9746b693ef235

Patch Set 1 #

Total comments: 13

Patch Set 2 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -231 lines) Patch
M core/include/fxge/fx_font.h View 2 chunks +25 lines, -17 lines 0 comments Download
M core/src/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp View 6 chunks +15 lines, -12 lines 0 comments Download
M core/src/fxge/ge/fx_ge_font.cpp View 1 4 chunks +63 lines, -142 lines 0 comments Download
M core/src/fxge/ge/text_int.h View 2 chunks +0 lines, -36 lines 0 comments Download
M xfa/src/fgas/src/font/fx_gefont.h View 1 chunk +1 line, -1 line 0 comments Download
M xfa/src/fgas/src/font/fx_gefont.cpp View 4 chunks +11 lines, -12 lines 0 comments Download
M xfa/src/fxbarcode/oned/BC_OneDimWriter.cpp View 1 3 chunks +4 lines, -7 lines 0 comments Download
M xfa/src/fxgraphics/src/fx_graphics.cpp View 1 3 chunks +5 lines, -4 lines 0 comments Download
M xfa/src/fxgraphics/src/fx_path_generator.h View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
Tom Sepez
RS LGTM
5 years, 4 months ago (2015-08-17 23:27:32 UTC) #1
Lei Zhang
A bit more work went into the merge. Requires review.
5 years, 4 months ago (2015-08-17 23:29:26 UTC) #2
Tom Sepez
Still LGTM, few nits. https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font.cpp File core/src/fxge/ge/fx_ge_font.cpp (right): https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font.cpp#newcode19 core/src/fxge/ge/fx_ge_font.cpp:19: FXFM_ENCODING_MS_SYMBOL, nit: indent seems off ...
5 years, 4 months ago (2015-08-17 23:49:29 UTC) #3
Lei Zhang
https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font.cpp File core/src/fxge/ge/fx_ge_font.cpp (right): https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font.cpp#newcode19 core/src/fxge/ge/fx_ge_font.cpp:19: FXFM_ENCODING_MS_SYMBOL, On 2015/08/17 23:49:29, Tom Sepez wrote: > nit: ...
5 years, 4 months ago (2015-08-18 01:36:40 UTC) #4
Lei Zhang
Committed patchset #2 (id:20001) manually as 1b0023986bc22ce362097b25deb9746b693ef235 (presubmit successful).
5 years, 4 months ago (2015-08-18 01:36:48 UTC) #5
Nico
5 years, 4 months ago (2015-08-18 02:19:34 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font...
File core/src/fxge/ge/fx_ge_font.cpp (right):

https://codereview.chromium.org/1297083002/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_font.cpp:19: FXFM_ENCODING_MS_SYMBOL,
On 2015/08/18 01:36:39, Lei Zhang wrote:
> On 2015/08/17 23:49:29, Tom Sepez wrote:
> > nit: indent seems off (2 vs 4 sp)?
> 
> git cl format did this. :\
> +thakis FYI

That's intentional. clang-format formats braced initializer lists like
constructors (because of uniform initialization in C++11), and constructors
would get 4 indent for its args too.

Powered by Google App Engine
This is Rietveld 408576698