Closed Bug 1540438 Opened 5 years ago Closed 5 years ago

Code clean-up: Remove tabs from suite code

Categories

(SeaMonkey :: General, enhancement)

enhancement
Not set
trivial

Tracking

(seamonkey2.49esr wontfix, seamonkey2.63 wontfix, seamonkey2.53 fixed, seamonkey2.57esr fixed)

RESOLVED FIXED
seamonkey2.65
Tracking Status
seamonkey2.49esr --- wontfix
seamonkey2.63 --- wontfix
seamonkey2.53 --- fixed
seamonkey2.57esr --- fixed

People

(Reporter: frg, Assigned: frg)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Even after Bug 1530955 we still have a lot of tabs and some trailing blanks left.

+++ This bug was initially created as a clone of Bug #1530955 +++

+++ This bug was initially created as a clone of Bug #1015741 +++

The language files for SeaMonkey contain extra whitespace in entities, at end of lines and at end of files, which makes them harder to parse/use in scripts. It would be great if this excess whitespace were removed.
(Probably also in other directories/file types, but if that's also wanted/needed it should happen in a separate bug.)

Summary: Code clean-up suite: Fix tabs and trailing spaces in suite → Code clean-up suite: Fix tabs in suite
Attachment #9054712 - Flags: review?(iann_bugzilla)
Attachment #9054712 - Flags: approval-comm-esr60?
Attachment #9054713 - Flags: review?(iann_bugzilla)
Attachment #9054713 - Flags: approval-comm-esr60?
Summary: Code clean-up suite: Fix tabs in suite → Code clean-up: Remove tabs from suite code
Blocks: 1540439
Comment on attachment 9054712 [details] [diff] [review]
1540438-1-suite-mailnews-whitespace.patch

>+++ b/suite/mailnews/components/addrbook/content/abCardViewOverlay.js
> function cvSetNode(node, text)
> {
>+  if ( node )
>+  {
>+    if ( !node.hasChildNodes() )
>+    {
>+      var textNode = document.createTextNode(text);
>+      node.appendChild(textNode);
>+    }
>+    else if ( node.childNodes.length == 1 )
>+      node.childNodes[0].nodeValue = text;
Whilst here can we remove the extra space within ()

>+    if ( text )
>+      visible = true;
>+    else
>+      visible = false;
Ditto

> function cvSetVisible(node, visible)
> {
>+  if ( visible )
Ditto

>+++ b/suite/mailnews/components/addrbook/content/addressbook.js

>+  var menuitem = top.document.getElementById(menuitemID);
>+  if ( menuitem )
Remove extra spaces in ()

>+  printEngineWindow = window.openDialog("chrome://messenger/content/msgPrintEngine.xul",
>+                    "",
>+                    "chrome,dialog=no,all",
>+                    1, [printUrl], statusFeedback, doPrintPreview, msgType);
Should be lined up with " in ("

>+++ b/suite/mailnews/content/mail3PaneWindowCommands.js
> var FolderPaneController =
> {
>+  supportsCommand: function(command)
>+  {
>+    switch ( command )
Remove extra spaces in ()

>+  isCommandEnabled: function(command)
>+  {
>+    switch ( command )
Ditto

>+      case "button_shiftDelete":
>+      if ( command == "cmd_delete" )
Ditto

>           if (folder.server.type == "nntp") {
>+             if ( command == "cmd_delete" ) {
Ditto

>+  doCommand: function(command)
>+  {
>     // if the user invoked a key short cut then it is possible that we got here for a command which is
>     // really disabled. kick out if the command should be disabled.
>     if (!this.isCommandEnabled(command)) return;
> 
>+    switch ( command )
Ditto

>+  supportsCommand: function(command)
>+  {
> 
>+    switch ( command )
Ditto

>   isCommandEnabled: function(command)
>   {
>     var enabled = new Object();
>     enabled.value = false;
>     var checkStatus = new Object();
> 
>     switch ( command )
Ditto

>       case "cmd_printpreview":
>+        if ( GetNumSelectedMessages() == 1 && gDBView)
Ditto

>+  onEvent: function(event)
>+  {
>+    // on blur events set the menu item texts back to the normal values
>+    if ( event == 'blur' )
Ditto
>         {
>             goSetMenuValue('cmd_undo', 'valueDefault');
>             goSetMenuValue('cmd_redo', 'valueDefault');
>         }
These four lines are not correctly indented.

> function GetNumSelectedMessages()
> {
>     try {
>         return gDBView.numSelected;
>     }
>     catch (ex) {
These seem to be incorrectly indented too.

> function SetFocusThreadPaneIfNotOnMessagePane()
> {
>   var focusedElement = WhichPaneHasFocus();
> 
>   if((focusedElement != GetThreadTree()) &&
>      (focusedElement != GetMessagePane()))
Missing space after if and next line will need indenting further after.

> function MsgNextMessage()
> {
>+  GoNextMessage(nsMsgNavigationType.nextMessage, false );
Extra space before )

>+++ b/suite/mailnews/content/messageWindow.js

>+  doCommand: function(command)
>+  {
>     // if the user invoked a key short cut then it is possible that we got here for a command which is
>     // really disabled. kick out if the command should be disabled.
>     if (!this.isCommandEnabled(command)) return;
> 
>     var navigationType = nsMsgNavigationType.nextUnreadMessage;
> 
>+    switch ( command )
Remove extra spaces within ()

>+++ b/suite/mailnews/content/msgMail3PaneWindow.js

>+    var numFrames = window.frames.length;
>+    for(var i = 0; i < numFrames; i++)
Missing space after for

r/a=me with those fixed.
Attachment #9054712 - Flags: review?(iann_bugzilla)
Attachment #9054712 - Flags: review+
Attachment #9054712 - Flags: approval-comm-esr60?
Attachment #9054712 - Flags: approval-comm-esr60+
Comment on attachment 9054713 [details] [diff] [review]
1540438-2-suite-whitespace.patch

>+++ b/suite/base/content/unix/platformCommunicatorOverlay.xul
>+  <!-- close -->
>+  <menuitem id="menu_close" label="&closeCmd.label;" key="key_close" accesskey="&closeCmd.accesskey;" command="cmd_close"/>
>+  <key id="key_close"  key="&closeCmd.key;" command="cmd_close" modifiers="accel"/>
>+  <key id="key_closeWindow"  key="&closeCmd.key;" command="cmd_closeWindow" modifiers="accel,shift"/>
Remove extra space before key=

>+  <key id="key_quit"  key="&quitApplicationCmd.key;" command="cmd_quit" modifiers="accel"/>
Remove extra space before key=

>+  <!-- Edit Menu -->
>+  <key id="key_redo"  key="&redoCmd.key;" command="cmd_redo" modifiers="accel"/>
Remove extra space before key=

r/a=me with those fixed.
Attachment #9054713 - Flags: review?(iann_bugzilla)
Attachment #9054713 - Flags: review+
Attachment #9054713 - Flags: approval-comm-esr60?
Attachment #9054713 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/e1343db925fa
Part 1: SeaMonkey tab removal in mail and news. r=IanN
https://hg.mozilla.org/comm-central/rev/0a8b3f8e67f7
Part 2: SeaMonkey tab removal in general suite parts. r=IanN

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.65

https://hg.mozilla.org/releases/comm-esr60/rev/8de0c91224f065369f21cada0e45f40a24607fc4
Part 1: SeaMonkey tab removal in mail and news. r=IanN a=IanN
https://hg.mozilla.org/releases/comm-esr60/rev/fbaf0fac419f1d4ab0abb0bcc0b6ca96f64b732c
Part 2: SeaMonkey tab removal in general suite parts. r=IanN a=IanN

Blocks: 1542542
Blocks: 1542636

Just noticed an ambiguous if/then indentation. I think this is what you intended (rather than braces around a larger chunk), not 100% sure.

Flags: needinfo?(frgrahl)
Comment on attachment 9056426 [details] [diff] [review]
1540438_indent_fixup.patch

Thanks Ben, you are right missed this one when cleaning up the patch. Fortunately only the indention is bad.

I will put it in later.
Flags: needinfo?(frgrahl)
Attachment #9056426 - Flags: review+
Attachment #9056426 - Flags: approval-comm-esr60+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/459233271154
Fix misleading if/then indent. r=frg
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: