Merge branch '222512-binary' into 'master'

Web IDE: Remove property `binary` on the file object.

See merge request gitlab-org/gitlab!42393

app/assets/javascripts/ide/components/ide_status_list.vue

@@ -2,7 +2,7 @@
 import { mapGetters } from 'vuex';
 import { GlLink, GlTooltipDirective } from '@gitlab/ui';
 import TerminalSyncStatusSafe from './terminal_sync/terminal_sync_status_safe.vue';
-import { getFileEOL } from '../utils';
+import { isTextFile, getFileEOL } from '~/ide/utils';
 
 export default {
   components: {
@@ -17,6 +17,9 @@ export default {
     activeFileEOL() {
       return getFileEOL(this.activeFile.content);
     },
+    activeFileIsText() {
+      return isTextFile(this.activeFile);
+    },
   },
 };
 </script>
@@ -30,7 +33,7 @@ export default {
         </gl-link>
       </div>
       <div>{{ activeFileEOL }}</div>
-      <div v-if="!activeFile.binary">{{ activeFile.editorRow }}:{{ activeFile.editorColumn }}</div>
+      <div v-if="activeFileIsText">{{ activeFile.editorRow }}:{{ activeFile.editorColumn }}</div>
       <div>{{ activeFile.fileLanguage }}</div>
     </template>
     <terminal-sync-status-safe />

app/assets/javascripts/ide/components/new_dropdown/upload.vue

@@ -28,14 +28,13 @@ export default {
       const { name } = file;
       const encodedContent = target.result.split('base64,')[1];
       const rawContent = encodedContent ? atob(encodedContent) : '';
-      const isText = isTextFile(rawContent, file.type, name);
+      const isText = isTextFile({ content: rawContent, mimeType: file.type, name });
 
       const emitCreateEvent = content =>
         this.$emit('create', {
           name: `${this.path ? `${this.path}/` : ''}${name}`,
           type: 'blob',
           content,
-          binary: !isText,
           rawPath: !isText ? target.result : '',
         });
 

app/assets/javascripts/ide/components/repo_editor.vue

@@ -14,7 +14,7 @@ import Editor from '../lib/editor';
 import FileTemplatesBar from './file_templates/bar.vue';
 import { __ } from '~/locale';
 import { extractMarkdownImagesFromEntries } from '../stores/utils';
-import { getPathParent, readFileAsDataURL, registerSchema } from '../utils';
+import { getPathParent, readFileAsDataURL, registerSchema, isTextFile } from '../utils';
 import { getRulesWithTraversal } from '../lib/editorconfig/parser';
 import mapRulesToMonaco from '../lib/editorconfig/rules_mapper';
 
@@ -60,7 +60,7 @@ export default {
     ]),
     ...mapGetters('fileTemplates', ['showFileTemplatesBar']),
     shouldHideEditor() {
-      return this.file && this.file.binary;
+      return this.file && !isTextFile(this.file);
     },
     showContentViewer() {
       return (

app/assets/javascripts/ide/lib/files.js

-import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
 import { decorateData, sortTree } from '../stores/utils';
 
 export const splitParent = path => {
@@ -13,13 +12,7 @@ export const splitParent = path => {
 /**
  * Create file objects from a list of file paths.
  */
-export const decorateFiles = ({
-  data,
-  tempFile = false,
-  content = '',
-  binary = false,
-  rawPath = '',
-}) => {
+export const decorateFiles = ({ data, tempFile = false, content = '', rawPath = '' }) => {
   const treeList = [];
   const entries = {};
 
@@ -68,7 +61,6 @@ export const decorateFiles = ({
     const fileFolder = parent && insertParent(parent);
 
     if (name) {
-      const previewMode = viewerInformationForPath(name);
       parentPath = fileFolder && fileFolder.path;
 
       file = decorateData({
@@ -79,7 +71,6 @@ export const decorateFiles = ({
         tempFile,
         changed: tempFile,
         content,
-        binary: (previewMode && previewMode.binary) || binary,
         rawPath,
         parentPath,
       });

app/assets/javascripts/ide/stores/actions.js

@@ -25,15 +25,7 @@ export const setResizingStatus = ({ commit }, resizing) => {
 
 export const createTempEntry = (
   { state, commit, dispatch, getters },
-  {
-    name,
-    type,
-    content = '',
-    binary = false,
-    rawPath = '',
-    openFile = true,
-    makeFileActive = true,
-  },
+  { name, type, content = '', rawPath = '', openFile = true, makeFileActive = true },
 ) => {
   const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
 
@@ -57,7 +49,6 @@ export const createTempEntry = (
     type,
     tempFile: true,
     content,
-    binary,
     rawPath,
   });
   const { file, parentPath } = data;
@@ -84,7 +75,6 @@ export const addTempImage = ({ dispatch, getters }, { name, rawPath = '' }) =>
     name: getters.getAvailableFileName(name),
     type: 'blob',
     content: rawPath.split('base64,')[1],
-    binary: true,
     rawPath,
     openFile: false,
     makeFileActive: false,

app/assets/javascripts/ide/stores/utils.js

@@ -23,7 +23,6 @@ export const dataStructure = () => ({
   staged: false,
   lastCommitSha: '',
   rawPath: '',
-  binary: false,
   raw: '',
   content: '',
   editorRow: 1,
@@ -49,7 +48,6 @@ export const decorateData = entity => {
     active = false,
     opened = false,
     changed = false,
-    binary = false,
     rawPath = '',
     file_lock,
     parentPath = '',
@@ -66,7 +64,6 @@ export const decorateData = entity => {
     active,
     changed,
     content,
-    binary,
     rawPath,
     file_lock,
     parentPath,

app/assets/javascripts/ide/utils.js

 import { languages } from 'monaco-editor';
-import { flatten } from 'lodash';
+import { flatten, isString } from 'lodash';
 import { SIDE_LEFT, SIDE_RIGHT } from './constants';
 
 const toLowerCase = x => x.toLowerCase();
@@ -42,15 +42,16 @@ const KNOWN_TYPES = [
   },
 ];
 
-export function isTextFile(content, mimeType, fileName) {
-  const knownType = KNOWN_TYPES.find(type => type.isMatch(mimeType, fileName));
+export function isTextFile({ name, content, mimeType = '' }) {
+  const knownType = KNOWN_TYPES.find(type => type.isMatch(mimeType, name));
 
   if (knownType) return knownType.isText;
 
   // does the string contain ascii characters only (ranges from space to tilde, tabs and new lines)
   const asciiRegex = /^[ -~\t\n\r]+$/;
+
   // for unknown types, determine the type by evaluating the file contents
-  return asciiRegex.test(content);
+  return isString(content) && (content === '' || asciiRegex.test(content));
 }
 
 export const createPathWithExt = p => {

spec/frontend/ide/components/ide_status_list_spec.js

@@ -75,7 +75,8 @@ describe('ide/components/ide_status_list', () => {
 
   describe('with binary file', () => {
     beforeEach(() => {
-      activeFile.binary = true;
+      activeFile.name = 'abc.dat';
+      activeFile.content = '🐱'; // non-ascii binary content
       createComponent();
     });
 

spec/frontend/ide/components/new_dropdown/upload_spec.js

@@ -85,7 +85,6 @@ describe('new dropdown upload', () => {
             name: textFile.name,
             type: 'blob',
             content: 'plain text',
-            binary: false,
             rawPath: '',
           });
         })
@@ -102,7 +101,6 @@ describe('new dropdown upload', () => {
         name: binaryFile.name,
         type: 'blob',
         content: binaryTarget.result.split('base64,')[1],
-        binary: true,
         rawPath: binaryTarget.result,
       });
     });

spec/frontend/ide/components/repo_editor_spec.js

@@ -45,7 +45,7 @@ describe('RepoEditor', () => {
 
   const createOpenFile = path => {
     const origFile = store.state.openFiles[0];
-    const newFile = { ...origFile, path, key: path };
+    const newFile = { ...origFile, path, key: path, name: 'myfile.txt', content: 'hello world' };
 
     store.state.entries[path] = newFile;
 
@@ -54,8 +54,9 @@ describe('RepoEditor', () => {
 
   beforeEach(() => {
     const f = {
-      ...file(),
+      ...file('file.txt'),
       viewMode: FILE_VIEW_MODE_EDITOR,
+      content: 'hello world',
     };
 
     const storeOptions = createStoreOptions();
@@ -142,6 +143,7 @@ describe('RepoEditor', () => {
           ...vm.file,
           projectId: 'namespace/project',
           path: 'sample.md',
+          name: 'sample.md',
           content: 'testing 123',
         });
 
@@ -200,7 +202,8 @@ describe('RepoEditor', () => {
 
     describe('when open file is binary and not raw', () => {
       beforeEach(done => {
-        vm.file.binary = true;
+        vm.file.name = 'file.dat';
+        vm.file.content = '🐱'; // non-ascii binary content
 
         vm.$nextTick(done);
       });
@@ -407,6 +410,9 @@ describe('RepoEditor', () => {
       beforeEach(done => {
         jest.spyOn(vm.editor, 'updateDimensions').mockImplementation();
         vm.file.viewMode = FILE_VIEW_MODE_PREVIEW;
+        vm.file.name = 'myfile.md';
+        vm.file.content = 'hello world';
+
         vm.$nextTick(done);
       });
 
@@ -650,7 +656,6 @@ describe('RepoEditor', () => {
             path: 'foo/foo.png',
             type: 'blob',
             content: 'Zm9v',
-            binary: true,
             rawPath: 'data:image/png;base64,Zm9v',
           });
         });

spec/frontend/ide/lib/files_spec.js

-import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
 import { decorateFiles, splitParent } from '~/ide/lib/files';
 import { decorateData } from '~/ide/stores/utils';
 
 const createEntries = paths => {
   const createEntry = (acc, { path, type, children }) => {
     const { name, parent } = splitParent(path);
-    const previewMode = viewerInformationForPath(name);
 
     acc[path] = {
       ...decorateData({
@@ -13,8 +11,6 @@ const createEntries = paths => {
         name,
         path,
         type,
-        previewMode,
-        binary: (previewMode && previewMode.binary) || false,
         parentPath: parent,
       }),
       tree: children.map(childName => expect.objectContaining({ name: childName })),

spec/frontend/ide/stores/actions/file_spec.js

@@ -241,7 +241,6 @@ describe('IDE store file actions', () => {
           200,
           {
             raw_path: 'raw_path',
-            binary: false,
           },
           {
             'page-title': 'testing getFileData',
@@ -305,7 +304,6 @@ describe('IDE store file actions', () => {
           200,
           {
             raw_path: 'raw_path',
-            binary: false,
           },
           {
             'page-title': 'testing old-dull-file',

spec/frontend/ide/stores/mutations/file_spec.js

@@ -61,13 +61,11 @@ describe('IDE store file mutations', () => {
       mutations.SET_FILE_DATA(localState, {
         data: {
           raw_path: 'raw',
-          binary: true,
         },
         file: localFile,
       });
 
       expect(localFile.rawPath).toBe('raw');
-      expect(localFile.binary).toBeTruthy();
       expect(localFile.raw).toBeNull();
       expect(localFile.baseRaw).toBeNull();
     });

spec/frontend/ide/utils_spec.js

@@ -13,60 +13,78 @@ import {
 
 describe('WebIDE utils', () => {
   describe('isTextFile', () => {
-    it('returns false for known binary types', () => {
-      expect(isTextFile('file content', 'image/png', 'my.png')).toBeFalsy();
-      // mime types are case insensitive
-      expect(isTextFile('file content', 'IMAGE/PNG', 'my.png')).toBeFalsy();
+    it.each`
+      mimeType        | name        | type        | result
+      ${'image/png'}  | ${'my.png'} | ${'binary'} | ${false}
+      ${'IMAGE/PNG'}  | ${'my.png'} | ${'binary'} | ${false}
+      ${'text/plain'} | ${'my.txt'} | ${'text'}   | ${true}
+      ${'TEXT/PLAIN'} | ${'my.txt'} | ${'text'}   | ${true}
+    `('returns $result for known $type types', ({ mimeType, name, result }) => {
+      expect(isTextFile({ content: 'file content', mimeType, name })).toBe(result);
     });
 
-    it('returns true for known text types', () => {
-      expect(isTextFile('file content', 'text/plain', 'my.txt')).toBeTruthy();
-      // mime types are case insensitive
-      expect(isTextFile('file content', 'TEXT/PLAIN', 'my.txt')).toBeTruthy();
-    });
+    it.each`
+      content                                   | mimeType                      | name
+      ${'{"éêė":"value"}'}                      | ${'application/json'}         | ${'my.json'}
+      ${'{"éêė":"value"}'}                      | ${'application/json'}         | ${'.tsconfig'}
+      ${'SELECT "éêė" from tablename'}          | ${'application/sql'}          | ${'my.sql'}
+      ${'{"éêė":"value"}'}                      | ${'application/json'}         | ${'MY.JSON'}
+      ${'SELECT "éêė" from tablename'}          | ${'application/sql'}          | ${'MY.SQL'}
+      ${'var code = "something"'}               | ${'application/javascript'}   | ${'Gruntfile'}
+      ${'MAINTAINER Александр "a21283@me.com"'} | ${'application/octet-stream'} | ${'dockerfile'}
+    `(
+      'returns true for file extensions that Monaco supports syntax highlighting for',
+      ({ content, mimeType, name }) => {
+        expect(isTextFile({ content, mimeType, name })).toBe(true);
+      },
+    );
 
-    it('returns true for file extensions that Monaco supports syntax highlighting for', () => {
-      // test based on both MIME and extension
-      expect(isTextFile('{"éêė":"value"}', 'application/json', 'my.json')).toBeTruthy();
-      expect(isTextFile('{"éêė":"value"}', 'application/json', '.tsconfig')).toBeTruthy();
-      expect(isTextFile('SELECT "éêė" from tablename', 'application/sql', 'my.sql')).toBeTruthy();
+    it('returns false if filename is same as the expected extension', () => {
+      expect(
+        isTextFile({
+          name: 'sql',
+          content: 'SELECT "éêė" from tablename',
+          mimeType: 'application/sql',
+        }),
+      ).toBeFalsy();
     });
 
-    it('returns true even irrespective of whether the mimes, extensions or file names are lowercase or upper case', () => {
-      expect(isTextFile('{"éêė":"value"}', 'application/json', 'MY.JSON')).toBeTruthy();
-      expect(isTextFile('SELECT "éêė" from tablename', 'application/sql', 'MY.SQL')).toBeTruthy();
-      expect(
-        isTextFile('var code = "something"', 'application/javascript', 'Gruntfile'),
-      ).toBeTruthy();
+    it('returns true for ASCII only content for unknown types', () => {
       expect(
-        isTextFile(
-          'MAINTAINER Александр "alexander11354322283@me.com"',
-          'application/octet-stream',
-          'dockerfile',
-        ),
+        isTextFile({
+          name: 'hello.mytype',
+          content: 'plain text',
+          mimeType: 'application/x-new-type',
+        }),
       ).toBeTruthy();
     });
 
-    it('returns false if filename is same as the expected extension', () => {
-      expect(isTextFile('SELECT "éêė" from tablename', 'application/sql', 'sql')).toBeFalsy();
-    });
-
-    it('returns true for ASCII only content for unknown types', () => {
-      expect(isTextFile('plain text', 'application/x-new-type', 'hello.mytype')).toBeTruthy();
+    it('returns false for non-ASCII content for unknown types', () => {
+      expect(
+        isTextFile({
+          name: 'my.random',
+          content: '{"éêė":"value"}',
+          mimeType: 'application/octet-stream',
+        }),
+      ).toBeFalsy();
     });
 
-    it('returns true for relevant filenames', () => {
-      expect(
-        isTextFile(
-          'MAINTAINER Александр "alexander11354322283@me.com"',
-          'application/octet-stream',
-          'Dockerfile',
-        ),
-      ).toBeTruthy();
+    it.each`
+      name            | result
+      ${'myfile.txt'} | ${true}
+      ${'Dockerfile'} | ${true}
+      ${'img.png'}    | ${false}
+      ${'abc.js'}     | ${true}
+      ${'abc.random'} | ${false}
+      ${'image.jpeg'} | ${false}
+    `('returns $result for $filename when no content or mimeType is passed', ({ name, result }) => {
+      expect(isTextFile({ name })).toBe(result);
     });
 
-    it('returns false for non-ASCII content for unknown types', () => {
-      expect(isTextFile('{"éêė":"value"}', 'application/octet-stream', 'my.random')).toBeFalsy();
+    it('returns true if content is empty string but false if content is not passed', () => {
+      expect(isTextFile({ name: 'abc.dat' })).toBe(false);
+      expect(isTextFile({ name: 'abc.dat', content: '' })).toBe(true);
+      expect(isTextFile({ name: 'abc.dat', content: '  ' })).toBe(true);
     });
   });