Uses editorconfig to maintain consistent coding styles #6627

ManasJayanth · 2015-11-12T19:33:47Z

Fix for #6626

yurydelendik · 2015-11-12T20:11:10Z

My stats show more than 130 files shall be affected, you diff has only 42.

stripCommentHeaders was not modified too.

timvandermeij · 2015-11-12T20:16:37Z

stripCommentHeaders from

pdf.js/make.js

Lines 606 to 615 in d8ddde2

    
           function stripCommentHeaders(content, filename) { 
        
             // Strip out all the vim/license headers. 
        
             var notEndOfComment = '(?:[^*]|\\*(?!/))+'; 
        
             var reg = new RegExp( 
        
               '\n/\\* -\\*- Mode' + notEndOfComment + '\\*/\\s*' + 
        
               '(?:/\\*' + notEndOfComment + '\\*/\\s*|//(?!#).*\n\\s*)+' + 
        
               '\'use strict\';', 'g'); 
        
             content = content.replace(reg, ''); 
        
             return content; 
        
           }

should also be removed, along with the calls to that function. As Yury mentioned, probably there are more files with the header, so use a search to find out which ones exactly.

yurydelendik · 2015-11-12T20:18:24Z

stripCommentHeaders ... should also be removed

Actually not removed, but modified to fit new header format.

timvandermeij · 2015-11-12T20:30:56Z

Right, that should be done. It seems that the files in /web are missing in this PR (example: https://github.com/mozilla/pdf.js/blob/master/web/annotations_layer_builder.js). They also have this header.

ManasJayanth · 2015-11-12T21:27:12Z

@yurydelendik Was it 131 files? 4 of them are from build directory.

yurydelendik · 2015-11-12T21:45:24Z

Was it 131 files

I think it was 132

examples/acroforms/forms.js
examples/helloworld/hello.js
examples/node/domparsermock.js
examples/node/domstubs.js
examples/node/getinfo.js
examples/node/pdf2svg.js
examples/svgviewer/viewer.js
extensions/chromium/contentscript.js
extensions/chromium/extension-router.js
extensions/chromium/feature-detect.js
extensions/chromium/options/options.js
extensions/chromium/pageAction/background.js
extensions/chromium/pdfHandler-v2.js
extensions/chromium/pdfHandler-vcros.js
extensions/chromium/pdfHandler.js
extensions/chromium/preserve-referer.js
extensions/chromium/restoretab.js
extensions/chromium/suppress-update.js
extensions/firefox/bootstrap.js
extensions/firefox/chrome/content.js
extensions/firefox/content/PdfJs-stub.jsm
extensions/firefox/content/PdfJs.jsm
extensions/firefox/content/PdfJsTelemetry-addon.jsm
extensions/firefox/content/PdfJsTelemetry.jsm
extensions/firefox/content/PdfStreamConverter.jsm
extensions/firefox/content/PdfjsChromeUtils.jsm
extensions/firefox/content/PdfjsContentUtils.jsm
extensions/firefox/content/pdfjschildbootstrap.js
extensions/firefox/tools/l10n.js
external/builder/builder.js
external/builder/test.js
external/crlfchecker/crlfchecker.js
external/crlfchecker/normtext.js
external/importL10n/locales.js
make.js
src/core/annotation.js
src/core/arithmetic_decoder.js
src/core/bidi.js
src/core/charsets.js
src/core/chunked_stream.js
src/core/cmap.js
src/core/colorspace.js
src/core/core.js
src/core/crypto.js
src/core/evaluator.js
src/core/font_renderer.js
src/core/fonts.js
src/core/function.js
src/core/glyphlist.js
src/core/image.js
src/core/jbig2.js
src/core/jpg.js
src/core/jpx.js
src/core/metrics.js
src/core/murmurhash3.js
src/core/network.js
src/core/obj.js
src/core/parser.js
src/core/pattern.js
src/core/pdf_manager.js
src/core/ps_parser.js
src/core/stream.js
src/core/worker.js
src/display/annotation_helper.js
src/display/api.js
src/display/canvas.js
src/display/font_loader.js
src/display/metadata.js
src/display/pattern_helper.js
src/display/svg.js
src/display/webgl.js
src/pdf.js
src/shared/cffStandardStrings.js
src/shared/fonts_utils.js
src/shared/util.js
src/worker_loader.js
test/downloadutils.js
test/driver.js
test/features/tests.js
test/features/worker-stub.js
test/font/fontutils.js
test/font/ttxdriver.js
test/test.js
test/testutils.js
test/unit/annotation_layer_spec.js
test/unit/api_spec.js
test/unit/cmap_spec.js
test/unit/crypto_spec.js
test/unit/evaluator_spec.js
test/unit/font_spec.js
test/unit/function_spec.js
test/unit/metadata_spec.js
test/unit/obj_spec.js
test/unit/parser_spec.js
test/unit/stream_spec.js
test/unit/testreporter.js
test/unit/ui_utils_spec.js
test/unit/util_spec.js
test/webbrowser.js
test/webserver.js
web/annotations_layer_builder.js
web/chromecom.js
web/compatibility.js
web/debugger.js
web/default_preferences.js
web/download_manager.js
web/firefoxcom.js
web/hand_tool.js
web/interfaces.js
web/mozPrintCallback_polyfill.js
web/overlay_manager.js
web/password_prompt.js
web/pdf_attachment_view.js
web/pdf_document_properties.js
web/pdf_find_bar.js
web/pdf_find_controller.js
web/pdf_history.js
web/pdf_link_service.js
web/pdf_outline_view.js
web/pdf_page_view.js
web/pdf_presentation_mode.js
web/pdf_rendering_queue.js
web/pdf_thumbnail_view.js
web/pdf_thumbnail_viewer.js
web/pdf_viewer.component.js
web/pdf_viewer.js
web/preferences.js
web/secondary_toolbar.js
web/text_layer_builder.js
web/ui_utils.js
web/view_history.js
web/viewer.js

yurydelendik · 2015-11-13T14:21:01Z

web/chromecom.js

@@ -1,5 +1,4 @@
-/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* Copyright 2013 Mozilla Foundation
+/* Copyright 2014 Mozilla Foundation


Can you revert changes of the year of copyright? It's hard to get it right and out of scope of this PR

Sorry about this Reverting

Snuffleupagus · 2015-11-13T17:23:33Z

It appears that this PR is still missing most of the files in https://github.com/mozilla/pdf.js/tree/master/extensions/firefox/content/.

Snuffleupagus · 2015-11-13T17:31:14Z

external/webL10n/l10n.js

@@ -1,5 +1,4 @@
-/**
- * Copyright (c) 2011-2013 Fabien Cazenave, Mozilla.
+/* Copyright 2014 Mozilla Foundation


Please revert this change; and also the rest of the changes of Copyright year in the rest of this patch.

yurydelendik · 2015-11-13T20:33:34Z

make.js

@@ -607,7 +605,6 @@ function stripCommentHeaders(content, filename) {
  // Strip out all the vim/license headers.
  var notEndOfComment = '(?:[^*]|\\*(?!/))+';
  var reg = new RegExp(
-    '\n/\\* -\\*- Mode' + notEndOfComment + '\\*/\\s*' +


add '\n' to the next line, we want to not affect very first comment

also can you fix the comment above -- it does not reflect the purpose of the code below

Removes the following as they unnecessary /* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ /* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */

yurydelendik · 2015-11-16T17:44:15Z

Hmm, grab-to-pan.js header is gone from viewer.js. @Rob--W is it okay or you prefer to keep it? We probably just need move copyright into main file header.

Rob--W · 2015-11-16T19:26:34Z

@yurydelendik I don't mind either way.

yurydelendik · 2015-11-16T19:51:39Z

Added comment about that at #3775 (maybe @prometheansacrifice can look into that later :)

Thank you for the patch.

Uses editorconfig to maintain consistent coding styles

timvandermeij added the other label Nov 12, 2015

yurydelendik reviewed Nov 13, 2015
View reviewed changes

Snuffleupagus reviewed Nov 13, 2015
View reviewed changes

timvandermeij assigned yurydelendik Nov 13, 2015

yurydelendik reviewed Nov 13, 2015
View reviewed changes

Uses editorconfig to maintain consistent coding styles

a2ba1b8

Removes the following as they unnecessary /* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ /* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */

yurydelendik added a commit that referenced this pull request Nov 16, 2015

Merge pull request #6627 from prometheansacrifice/use-editorconfig

99f9b52

Uses editorconfig to maintain consistent coding styles

yurydelendik merged commit 99f9b52 into mozilla:master Nov 16, 2015

timvandermeij mentioned this pull request Nov 16, 2015

Use .editorconfig and remove per file configuration. #6626

Closed

Snuffleupagus mentioned this pull request Nov 21, 2015

Make stripCommentHeaders less greedy, to ensure that it doesn't eat 'use strict' directive at the top of files (PR 6627 follow-up) #6671

Merged

ManasJayanth mentioned this pull request Nov 27, 2015

Create a Bower Style Guide bower/bower#2043

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uses editorconfig to maintain consistent coding styles #6627

Uses editorconfig to maintain consistent coding styles #6627

ManasJayanth commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

timvandermeij commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

timvandermeij commented Nov 12, 2015

ManasJayanth commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

yurydelendik Nov 13, 2015

ManasJayanth Nov 13, 2015

Snuffleupagus commented Nov 13, 2015

Snuffleupagus Nov 13, 2015

yurydelendik Nov 13, 2015

yurydelendik commented Nov 16, 2015

Rob--W commented Nov 16, 2015

yurydelendik commented Nov 16, 2015

Uses editorconfig to maintain consistent coding styles #6627

Uses editorconfig to maintain consistent coding styles #6627

Conversation

ManasJayanth commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

timvandermeij commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

timvandermeij commented Nov 12, 2015

ManasJayanth commented Nov 12, 2015

yurydelendik commented Nov 12, 2015

yurydelendik Nov 13, 2015

Choose a reason for hiding this comment

ManasJayanth Nov 13, 2015

Choose a reason for hiding this comment

Snuffleupagus commented Nov 13, 2015

Snuffleupagus Nov 13, 2015

Choose a reason for hiding this comment

yurydelendik Nov 13, 2015

Choose a reason for hiding this comment

yurydelendik commented Nov 16, 2015

Rob--W commented Nov 16, 2015

yurydelendik commented Nov 16, 2015