813: Non C locale patches fail
- Fixed
- Review Board
| lukasz*******@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Jan. 30, 2009 | |
| 822 |
Review board fails when trying to submint a patch created with diff in non
C locale.
Example patch:
--- DirtyDock.h (wersja 13191)
+++ DirtyDock.h (kopia robocza)
@@ -48,6 +48,9 @@
QAction* centerZoomAction;
std::vector<MapFeature*> Selection;
+public:
+ void changeEvent(QEvent*);
+ void retranslateUi();
};
#endif
What steps will reproduce the problem?
1. click New Review Request
2. Insert the above diff into a file a add it to the "Diff:" selector
3. click "Create Review Request"
What is the expected output? What do you see instead?
The patch should be sent and accepted and the usual stuff should follow,
instead I get back to r/new/ with and error "Unable to parse diff revision
header '(wersja 13191)'"
What are you using to generate the patch? Unfortunately, this is just sort of how it has to be. Most applications that take patches with revision info need them in C locale, because it's not really feasible to support every language under the sun in the diff parser.
-
+ NeedInfo
svn diff on http://svn.openstreetmap.org/applications/editors/merkaartor deejay1@sulaco:~$ locale LANG=pl_PL.UTF-8 LC_CTYPE="pl_PL.UTF-8" LC_NUMERIC="pl_PL.UTF-8" LC_TIME="pl_PL.UTF-8" LC_COLLATE="pl_PL.UTF-8" LC_MONETARY="pl_PL.UTF-8" LC_MESSAGES="pl_PL.UTF-8" LC_PAPER="pl_PL.UTF-8" LC_NAME="pl_PL.UTF-8" LC_ADDRESS="pl_PL.UTF-8" LC_TELEPHONE="pl_PL.UTF-8" LC_MEASUREMENT="pl_PL.UTF-8" LC_IDENTIFICATION="pl_PL.UTF-8" LC_ALL= Maybe instead of checking (now I'm just guessing) for "(revision 1234)" it could just see if there are "()" and numbers inside as I suppose that for different locales the format is the same only the "revision" string changes...
It surprises me that a tool to generate data interchange files (svn diff) would localize the internal diff markers (much like 'diff' doesn't consult the user's preferred date format when printing the file timestamp). It seems like either 1) svn shouldn't be localizing diff output, or 2) svn should have a simple command-line way to override that behavior. It is somewhat sad that the diff format (or svn's extension of that) cannot support separate per-chunk or per-file metadata. You (lukasz) are right that rb does search for '(revision ...)', and that's probably the easiest restriction to fix (as long as the parsing code is strict enough to require it be at the end of the path).
Should be fixed in r1722, in that we generate SVN diffs now with the C locale. We still don't support diffs with non-C locales, but post-review should generate proper diffs.
-
- NeedInfo + Fixed
-
+ Milestone-Release1.0 + Component-Scripts