Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent newlines for singleline Code editor #2704

Merged
merged 3 commits into from Oct 13, 2020
Merged

Prevent newlines for singleline Code editor #2704

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03099a7
Prevent newlines for singleline editors
Oct 8, 2020
2cd51a0
Merge branch 'develop' into fix-single-line-input-newlines
Oct 10, 2020
a3113d6
Merge branch 'develop' into fix-single-line-input-newlines
develohpanda Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 9 additions & 7 deletions packages/insomnia-app/app/ui/components/codemirror/code-editor.js
View file
Open in desktop
Expand Up @@ -795,18 +795,20 @@ class CodeEditor extends React.Component {

_codemirrorValueBeforeChange(doc, change) {
const value = this.codeMirror.getDoc().getValue();

// If we're in single-line mode, merge all changed lines into one
if (this.props.singleLine && change.text && change.text.length > 1) {
const text = change.text
.join('') // join all changed lines into one
.replace(/\n/g, ' '); // Convert all whitespace to spaces
change.update(change.from, change.to, [text]);
}

// Suppress lint on empty doc or single space exists (default value)
if (value.trim() === '') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A newline value when trimmed will be an empty string so old code to detect multilines is skipped. So just moved code up to allow processing so no net new code 馃挭
image

this._codemirrorSmartSetOption('lint', false);
} else {
this._codemirrorSmartSetOption('lint', this.props.lintOptions || true);
// If we're in single-line mode, merge all changed lines into one
if (this.props.singleLine && change.text && change.text.length > 1) {
const text = change.text
.join('') // join all changed lines into one
.replace(/\n/g, ' '); // Convert all whitespace to spaces
change.update(change.from, change.to, [text]);
}

// Don't allow non-breaking spaces because they break the GraphQL syntax
if (doc.options.mode === 'graphql' && change.text && change.text.length > 1) {
Comment on lines 813 to 814
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this condition also needs to be moved out, for the same reason? The change.text.length > 1 condition is the same, and looking through the logic the graphql editor seems like it might exhibit the same buggy behavior when first entering text.

It doesn't relate directly to the issue this PR fixes so I would suggest doing it in a separate PR if it does need to be moved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I'm not familiar with the graphql side of things so it wasn't something I tested. But definitely happy to test it and submit a PR if you know how and when that doc.options.mode == 'graphql' would get triggered 馃槂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example workspace with a request containing the special character after the "title" text (see screenshot below).

Run in Insomnia}

I think the intended behavior of this code is to replace the special space with a regular space. If you change the text in the request body, I'm guessing it should clear the error by replacing the character 馃

image

Also, on Windows you can add that special character with Alt 255 (hold alt, and type 255 on the numpad)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Managed to replicate the problem and PR to fix it is here #2770 馃挭

Expand Down