Skip to content

Commit d0331f4

Browse files
bjones1bjones1
committed
Fix: consistent EOL handling of source code.
1 parent 2b0c31d commit d0331f4

File tree

2 files changed

+48
-98
lines changed

2 files changed

+48
-98
lines changed

‎server/src/translation.rs‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ pub async fn translation_task(
415415

416416
// Leave space for a server message during the init phase.
417417
let mut id: f64 = INITIAL_MESSAGE_ID + MESSAGE_ID_INCREMENT;
418+
// The source code, provided by the IDE. It will use whatever the IDE provides for EOLs, which is stored in `eol` below.
418419
let mut source_code = String::new();
419420
let mut code_mirror_doc = String::new();
420421
// The initial state will be overwritten by the first `Update`
@@ -530,45 +531,38 @@ pub async fn translation_task(
530531
// this is a PDF file. (TODO: look at the
531532
// magic number also -- "%PDF").
532533
let use_pdf_js = http_request.file_path.extension() == Some(OsStr::new("pdf"));
533-
let (simple_http_response, option_update, file_contents) = match file_contents_option {
534+
let ((simple_http_response, option_update), file_contents) = match file_contents_option {
534535
Some(file_contents) => {
535536
// If there are Windows newlines, replace
536537
// with Unix; this is reversed when the
537538
// file is sent back to the IDE.
538-
eol = find_eol_type(&file_contents);
539-
let file_contents = if use_pdf_js { file_contents } else { file_contents.replace("\r\n", "\n") };
540-
file_to_response(&http_request, &current_file, Some(file_contents), use_pdf_js).await
539+
(file_to_response(&http_request, &current_file, Some(&file_contents), use_pdf_js).await, file_contents)
541540
},
542541
None => {
543542
// The file wasn't available in the IDE.
544543
// Look for it in the filesystem.
545544
match File::open(&http_request.file_path).await {
546545
Err(err) => (
547-
SimpleHttpResponse::Err(SimpleHttpResponseError::Io(err)),
548-
None,
549-
None
546+
(
547+
SimpleHttpResponse::Err(SimpleHttpResponseError::Io(err)),
548+
None,
549+
),
550+
// There's no file, so return empty contents, which will be ignored.
551+
"".to_string()
550552
),
551553
Ok(mut fc) => {
552554
let option_file_contents = try_read_as_text(&mut fc).await;
553-
let option_file_contents = if let Some(file_contents) = option_file_contents {
554-
eol = find_eol_type(&file_contents);
555-
let file_contents = if use_pdf_js { file_contents } else { file_contents.replace("\r\n", "\n") };
556-
Some(file_contents)
557-
} else {
558-
None
559-
};
560-
// <a id="binary-file-sniffer"></a>If this
561-
// is a binary file (meaning we can't read
562-
// the contents as UTF-8), send the
563-
// contents as none to signal this isn't a
564-
// text file.
565-
file_to_response(
566-
&http_request,
567-
&current_file,
568-
option_file_contents,
569-
use_pdf_js,
555+
(
556+
file_to_response(
557+
&http_request,
558+
&current_file,
559+
option_file_contents.as_ref(),
560+
use_pdf_js,
561+
)
562+
.await,
563+
// If the file is binary, return empty contents, which will be ignored.
564+
option_file_contents.unwrap_or("".to_string())
570565
)
571-
.await
572566
}
573567
}
574568
}
@@ -582,9 +576,10 @@ pub async fn translation_task(
582576
error!("Not plain!");
583577
break;
584578
};
579+
source_code = file_contents;
580+
eol = find_eol_type(&source_code);
585581
// We must clone here, since the original
586582
// is placed in the TX queue.
587-
source_code = file_contents.unwrap();
588583
code_mirror_doc = plain.doc.clone();
589584
code_mirror_doc_blocks = Some(plain.doc_blocks.clone());
590585
sync_state = SyncState::Pending(id);
@@ -841,15 +836,17 @@ pub async fn translation_task(
841836
Some(cfw) => match codechat_for_web_to_source(
842837
&cfw)
843838
{
844-
Ok(result) => {
839+
Ok(new_source_code) => {
840+
// Correct EOL endings for use with the IDE.
841+
let new_source_code_eol = eol_convert(new_source_code, &eol);
845842
let ccfw = if sync_state == SyncState::InSync && allow_source_diffs {
846843
Some(CodeChatForWeb {
847844
metadata: cfw.metadata,
848845
source: CodeMirrorDiffable::Diff(CodeMirrorDiff {
849846
// Diff with correct EOLs, so that (for
850847
// CRLF files as well as LF files) offsets
851848
// are correct.
852-
doc: diff_str(&eol_convert(source_code, &eol), &eol_convert(result.clone(), &eol)),
849+
doc: diff_str(&source_code, &new_source_code_eol),
853850
doc_blocks: vec![],
854851
}),
855852
})
@@ -859,14 +856,12 @@ pub async fn translation_task(
859856
source: CodeMirrorDiffable::Plain(CodeMirror {
860857
// We must clone here, so that it can be
861858
// placed in the TX queue.
862-
doc: eol_convert(result.clone(), &eol),
859+
doc: new_source_code_eol.clone(),
863860
doc_blocks: vec![],
864861
}),
865862
})
866863
};
867-
// Store the document with Unix-style EOLs
868-
// (LFs).
869-
source_code = result;
864+
source_code = new_source_code_eol;
870865
let CodeMirrorDiffable::Plain(cmd) = cfw.source else {
871866
// TODO: support diffable!
872867
error!("No diff!");

‎server/src/webserver.rs‎

Lines changed: 21 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -666,44 +666,6 @@ pub async fn filesystem_endpoint(
666666
}
667667
}
668668

669-
// Use the provided HTTP request to look for the requested file, returning it as
670-
// an HTTP response. This should be called from within a processing task.
671-
pub async fn make_simple_http_response(
672-
// The HTTP request presented to the processing task.
673-
http_request: &ProcessingTaskHttpRequest,
674-
// Path to the file currently being edited.
675-
current_filepath: &Path,
676-
// True to use the PDF.js viewer for this file.
677-
use_pdf_js: bool,
678-
) -> (
679-
// The response to send back to the HTTP endpoint.
680-
SimpleHttpResponse,
681-
// If this file is currently being edited, this is the body of an `Update`
682-
// message to send.
683-
Option<UpdateMessageContents>,
684-
// The resulting file contents, if this is a CodeChat Editor file
685-
Option<String>,
686-
) {
687-
// Convert the provided URL back into a file name.
688-
let file_path = &http_request.file_path;
689-
690-
// Read the file
691-
match File::open(file_path).await {
692-
Err(err) => (
693-
SimpleHttpResponse::Err(SimpleHttpResponseError::Io(err)),
694-
None,
695-
None,
696-
),
697-
Ok(mut fc) => {
698-
let file_contents = try_read_as_text(&mut fc).await;
699-
// <a id="binary-file-sniffer"></a>If this is a binary file (meaning
700-
// we can't read the contents as UTF-8), send the contents as none
701-
// to signal this isn't a text file.
702-
file_to_response(http_request, current_filepath, file_contents, use_pdf_js).await
703-
}
704-
}
705-
}
706-
707669
// Determine if the provided file is text or binary. If text, return it as a
708670
// Unicode string. If binary, return None.
709671
pub async fn try_read_as_text(file: &mut File) -> Option<String> {
@@ -732,7 +694,7 @@ pub async fn file_to_response(
732694
// `try_canonicalize`.
733695
current_filepath: &Path,
734696
// Contents of this file, if it's text; None if it was binary data.
735-
file_contents: Option<String>,
697+
file_contents: Option<&String>,
736698
// True to use the PDF.js viewer for this file.
737699
use_pdf_js: bool,
738700
) -> (
@@ -742,8 +704,6 @@ pub async fn file_to_response(
742704
// populate the Client with the parsed `file_contents`. In all other cases,
743705
// return None.
744706
Option<UpdateMessageContents>,
745-
// The `file_contents` if this is a
746-
Option<String>,
747707
) {
748708
// Use a lossy conversion, since this is UI display, not filesystem access.
749709
let file_path = &http_request.file_path;
@@ -753,7 +713,6 @@ pub async fn file_to_response(
753713
file_path.to_path_buf(),
754714
)),
755715
None,
756-
None,
757716
);
758717
};
759718
let name = escape_html(&file_name.to_string_lossy());
@@ -771,7 +730,6 @@ pub async fn file_to_response(
771730
codechat_editor_js_name,
772731
)),
773732
None,
774-
None,
775733
);
776734
};
777735
let codechat_editor_css_name = format!("CodeChatEditor{js_test_suffix}.css");
@@ -781,28 +739,32 @@ pub async fn file_to_response(
781739
codechat_editor_css_name,
782740
)),
783741
None,
784-
file_contents,
785742
);
786743
};
787744

788745
// Compare these files, since both have been canonicalized by
789746
// `try_canonical`.
790747
let is_current_file = file_path == current_filepath;
791748
let is_toc = http_request.flags == ProcessingTaskHttpRequestFlags::Toc;
792-
let (translation_results_string, path_to_toc) =
793-
if let Some(ref file_contents_text) = file_contents {
794-
if is_current_file || is_toc {
795-
source_to_codechat_for_web_string(file_contents_text, file_path, is_toc)
796-
} else {
797-
// If this isn't the current file, then don't parse it.
798-
(TranslationResultsString::Unknown, None)
799-
}
800-
} else {
801-
(
802-
TranslationResultsString::Binary,
803-
find_path_to_toc(file_path),
749+
let (translation_results_string, path_to_toc) = if let Some(file_contents_text) = file_contents
750+
{
751+
if is_current_file || is_toc {
752+
source_to_codechat_for_web_string(
753+
// Ensure we work with Unix-style (LF only) files, since other line endings break the translation process.
754+
&file_contents_text.replace("\r\n", "\n"),
755+
file_path,
756+
is_toc,
804757
)
805-
};
758+
} else {
759+
// If this isn't the current file, then don't parse it.
760+
(TranslationResultsString::Unknown, None)
761+
}
762+
} else {
763+
(
764+
TranslationResultsString::Binary,
765+
find_path_to_toc(file_path),
766+
)
767+
};
806768
let is_project = path_to_toc.is_some();
807769
// For project files, add in the sidebar. Convert this from a Windows path
808770
// to a Posix path if necessary.
@@ -834,7 +796,6 @@ pub async fn file_to_response(
834796
file_name,
835797
))),
836798
None,
837-
None,
838799
);
839800
};
840801
return (
@@ -854,14 +815,13 @@ pub async fn file_to_response(
854815
},
855816
),
856817
None,
857-
None,
858818
);
859819
}
860820

861821
let codechat_for_web = match translation_results_string {
862822
// The file type is binary. Ask the HTTP server to serve it raw.
863823
TranslationResultsString::Binary => return
864-
(SimpleHttpResponse::Bin(file_path.to_path_buf()), None, None)
824+
(SimpleHttpResponse::Bin(file_path.to_path_buf()), None)
865825
,
866826
// The file type is unknown. Serve it raw.
867827
TranslationResultsString::Unknown => {
@@ -871,12 +831,11 @@ pub async fn file_to_response(
871831
mime_guess::from_path(file_path).first_or_text_plain(),
872832
),
873833
None,
874-
None
875834
);
876835
}
877836
// Report a lexer error.
878837
TranslationResultsString::Err(err_string) => {
879-
return (SimpleHttpResponse::Err(SimpleHttpResponseError::LexerError(err_string)), None, None);
838+
return (SimpleHttpResponse::Err(SimpleHttpResponseError::LexerError(err_string)), None);
880839
}
881840
// This is a CodeChat file. The following code wraps the CodeChat for
882841
// web results in a CodeChat Editor Client webpage.
@@ -904,7 +863,6 @@ pub async fn file_to_response(
904863
</html>"#,
905864
)),
906865
None,
907-
None
908866
);
909867
}
910868
};
@@ -916,7 +874,6 @@ pub async fn file_to_response(
916874
file_path.to_path_buf(),
917875
)),
918876
None,
919-
None,
920877
);
921878
};
922879
let dir = path_display(raw_dir);
@@ -926,7 +883,6 @@ pub async fn file_to_response(
926883
file_path.to_path_buf(),
927884
)),
928885
None,
929-
None,
930886
);
931887
};
932888
// Build and return the webpage.
@@ -973,7 +929,6 @@ pub async fn file_to_response(
973929
cursor_position: None,
974930
scroll_position: None,
975931
}),
976-
file_contents,
977932
)
978933
}
979934

0 commit comments

Comments
 (0)