1
\$\begingroup\$

I'm not a JavaScript programmer. I have next to no idea about "what's out there" of libraries and so on. Therefore it's not unlikely that the referred to Windows HTML Application (HTA) code here, which is a mix of HTML5, CSS3 and JScript 5.6, may be incompatible with such libraries, common naming conventions, whatever, and I would like to rectify the issues that the more JavaScript-experienced reader can spot.

Compatibility is important because this is intended as a host environment for simple JavaScript scripts, which may likely use common libraries etc.

This code consists of a number of logical modules encapsulated in a single .HTA file by design*.

Therefore, even though it's only about half-finished it's too large to present inline here, I think, at about 750 lines. And since I have no idea what the issues could be, I'm reluctant to pare it down to what I think can be relevant. I think would be bound to err there.

Code at http://pastebin.com/FW88DLS1.

Oh, in order to make sense of the code it might help to see how the app looks:

enter image description here


As per request in comments I tried to post the full code inline here, but there was a limitation of 30.000 characters, while the code is some 36.000 characters.

Here's the CSS part (first part of the code):

<html>
    <!-- Windows HTA application "Text Stream Host". Copyright (©) 2014 Alf P. Steinbach -->
    <head id="head-element">
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
        <meta http-equiv="X-UA-Compatible" content="IE=edge">
        <meta http-equiv="MSThemeCompatible" content="yes">

        <title id="title-element"></title>

        <style>
            * { font: 10pt 'MS Shell Dlg 2'; }

            code { font: 10pt 'Courier new'; }

            body {
                padding: 0; margin: 0; overflow: hidden;
                background-color: #F0F0F0;
            }

            #text-stream-display p { margin: 0; padding: 0; color: #000080; }

            #control-area {
                padding-top: 0; padding-left: 0; padding-right: 0.5em; padding-bottom: 0;
                overflow: none;
                margin: 0;
                position: absolute;
                left: 0; top: 0;
                /*background-color: #FF0000;*/
                width: 30em; height: 2em; left: -14em; top: +14em;
                transform: rotate(-90deg);
            }

            #info-area {
                background-color: white;
                overflow: none;
                position: absolute;
                left: 2.5em; top: 0; right: 0; bottom: 0px;
            }

            #commandline-display  {
                color: gray;
                overflow: auto;
                white-space: nowrap;
                padding: 0.5em;
                border-bottom: 1px solid gray;
                margin-bottom: 0.5em;
                position: absolute; left: 0; top: 0; right: 0;
                height: 7em;
            }

            .command-line { background-color: #F0FFF0; }

            #text-stream-display  {
                overflow: auto;
                white-space: nowrap;
                padding: 0.5em;
                position: absolute;
                left: 0; top: 0px; right: 0; bottom: 0px;
            }

            .checkbox-div {
                border: 1px black solid;
                border-radius: 5px;
                padding-bottom: 0.2em; margin-bottom: 0.1em;
                padding-right: 0.5em;
                display: inline-block;
                cursor: pointer;
            }

            .checkbox-div:hover {
                background: #D0E0FF;
            }
        </style>

Here's the first logical JavaScript module (following right after the above in the full code):

        <!-- Namespace          js_util
                                JavaScript utilities -->
        <script type="text/javascript">
            js_util = new function()
            {
                this.is_whitespace_character = function( ch )
                {
                    return (ch == ' ' || ch == '\n' || ch == '\r' || ch == '\t');
                };

                this.is_whitespace = function( s )
                {
                    with( this )
                    {
                        var n = s.length;
                        for( var i = 0; i < n; ++i )
                        {
                            if( !is_whitespace_character( s.substr( i, 1 ) ) )
                            {
                                return false;
                            }
                        }
                    }
                    return true;
                };

                this.hex_digits = '0123456789ABCDEF';

                this.hex_from_int = function( number, n_digits )
                {
                    if( n_digits == undefined ) { n_digits = 8; }
                    var digits = new Array();
                    for( var i = 1;  i <= n_digits;  ++i )
                    {
                        digits.push( number & 0xF );
                        number >>>= 4;
                    }
                    var result = '';
                    for( var i = n_digits - 1; i >= 0; --i )
                    {
                        result += this.hex_digits.substr( digits[i], 1 );
                    }
                    return result;
                }

                this.add_class = function( html_elem, classname )
                {
                    var classname_set = html_elem.classList;
                    if( !classname_set.contains( classname ) )
                    {
                        classname_set.toggle( classname );
                        return true;
                    }
                    return false;
                }

                this.unquoted = function( s )
                {
                    var quote = '"';
                    var n = s.length;
                    if( n <= 1 )
                    {
                        return s;
                    }
                    if( s.substr( 0, 1 ) == quote && s.substr( n - 1, 1 ) == quote )
                    {
                        return s.substring( 1, n - 1 );
                    }
                    return s;
                }

                this.local_file_url = function( filespec )
                {
                    with( this )
                    {
                        var url = 'file:///' + unquoted( filespec ).replace( /\\/g, '/' );
                        return url;
                    }
                }
            };
        </script>

Here's the second JavaScript module:

        <!-- Namespace      windows.wsh
                            Windows Script Host (shell functionality only). -->
        <script type="text/javascript">
            var windows = windows || {};
            windows.wsh = new function()
            {
                this.shell = new ActiveXObject( 'WScript.Shell' );

                this.sleep = function( millisecs )
                {
                    with( this )
                    {
                        var nowindow = 0;
                        var wait = true;
                        shell.Run( 'ping 1.1.1.1 -n 1 -w ' + millisecs, nowindow, wait );
                    }
                };

                this.is_running = function( wsh_execution )
                {
                    return (wsh_execution.Status == 0);
                };

                this.run_hidden = function( command )
                {
                    var hidden_window = 0;
                    var wait_for_completion = true;
                    with( this )
                    {
                        return shell.Run( command, hidden_window, wait_for_completion );
                    }
                }
            };
        </script>

Here's the third JavaScript module:

        <!-- Namespace      windows.fs
                            File System Object (part of Window's script support). -->
        <script type="text/javascript">
            var windows = windows || {};
            windows.fs = new function()
            {
                this.fso = new ActiveXObject( "Scripting.FileSystemObject" );

                this.tempfolder        = this.fso.GetSpecialFolder( 2 );
                this.tempfolder_path   = this.tempfolder.Path;

                this.open_for_reading = function( textfile_spec )
                {
                    var for_reading         = 1;

                    var do_not_create       = false;

                    var ascii_encoding      = 0;
                    var utf16_encoding      = -1;
                    var default_encoding    = -2;

                    with( this )
                    {
                        return fso.OpenTextFile(
                            textfile_spec,
                            for_reading, do_not_create, default_encoding
                            );
                    }
                }

                this.joined_paths = function( folder_path, filename )
                {
                    with( this ) return fso.BuildPath( folder_path, filename );
                }

                this.temp_filename = function()
                {
                    with( this ) return fso.GetTempName();
                }

                this.temp_filepath = function()
                {
                    with( this ) return joined_paths( tempfolder_path, temp_filename() );
                }

                this.quoted = function( path )
                {
                    var quote = '"';
                    var is_quoted = (path.length > 0 && path.substr( 0, 1 ) == quote);
                    return (is_quoted? path : quote + path + quote);
                }
            };
        </script>

Here's the fourth JavaScript module:

        <!-- Namespace      windows.process_info
                            Windows process information. -->
        <script type="text/javascript">
            var windows = windows || {};
            windows.process_info = new function()
            {
                var pg = {};    // "private globals" namespace, to make that explicit.

                pg.Record = function()
                {
                    this._add_item = function( spec )
                    {
                        i_delimiter = spec.indexOf( '=' );
                        if( i_delimiter != -1 )
                        {
                            var property_name = spec.substring( 0, i_delimiter );
                            var value = spec.substr( i_delimiter + 1 );
                                this[property_name] = value;
                        }
                    }
                };

                this.new_records = function()
                {
                    var filepath = windows.fs.temp_filepath();
                    var exitcode = windows.wsh.run_hidden(
                        'cmd /c wmic process list full >' + windows.fs.quoted( filepath )
                        );
                    if( exitcode != 0 )
                    {
                        return null;        // TODO: exception
                    }

                    var records = new Array();
                    var current_record = null;

                    var f = windows.fs.open_for_reading( filepath );
                    while( !f.AtEndOfStream )
                    {
                        var line = f.ReadLine();

                        var i_end = line.length - 1;
                        while( i_end >= 0 && line.substr( i_end, 1 ) < ' ' )
                        {
                            --i_end;
                        }
                        if( 0 <= i_end && i_end < line.length - 1 )
                        {
                            line = line.substr( 0, i_end + 1 );
                        }

                        if( js_util.is_whitespace( line ) )
                        {
                            if( current_record != null )
                            {
                                records.push( current_record );
                                current_record = null;
                            }
                        }
                        else //  line.length > 0
                        {
                            if( current_record == null )
                            {
                                current_record = new pg.Record();
                            }
                            current_record._add_item( line );
                        }
                    }

                    // Complete a possible last record.
                    if( current_record != null )
                    {
                        records.push( current_record );
                    }

                    f.Close();
                    windows.fs.fso.DeleteFile( filepath );

                    return records;
                };
            };
        </script>

When I try to post the fifth and main JavaScript module, I get an error about exceeding the max length. So that module plus the main HTML body is missing here. Consult the PasteBin posting for a complete version that can be tried out – which I believe is a practical necessity for gaining a good understanding of code.


*Single file: the idea is to provide a simple text only i/o host environment for learning programming, where one simply drags the javascript source onto the HTA file. And for beginners I think it's decidedly an advantage to have the host environment as a single file, with no special installation issues etc.

\$\endgroup\$
6
  • \$\begingroup\$ Have you considered jsbin.com, or jsfiddle.net, or plnkr.co instead of writing your own environment? \$\endgroup\$
    – konijn
    Commented Apr 4, 2014 at 19:21
  • \$\begingroup\$ @konijn: Thanks for those references. Are they local execution environments? Needs to be local. \$\endgroup\$ Commented Apr 4, 2014 at 19:24
  • \$\begingroup\$ No, why do they need to be local? \$\endgroup\$
    – konijn
    Commented Apr 4, 2014 at 19:28
  • \$\begingroup\$ @konijn: usually that's because of connectivity issues. for corporate work there's also security aspects. \$\endgroup\$ Commented Apr 4, 2014 at 19:29
  • \$\begingroup\$ @AlfP.Steinbach jsbin is open source github.com/jsbin/jsbin so is plnkr github.com/filearts/plunker \$\endgroup\$
    – megawac
    Commented Apr 5, 2014 at 5:36

2 Answers 2

5
\$\begingroup\$

A few things immediately caught my eye:

  • You're using a weird construct: new function() {}. The function(){} expression already creates a new function, but in addition to that you're using it with new to invoke it as a constructor for new object. The way you're using it, you could easily replace it with just plain object literal. For example in js_util:

    js_util = {
        isWhitespaceCharacter: function(ch) {
            return (ch == ' ' || ch == '\n' || ch == '\r' || ch == '\t');
        },
        ...
    };
    

    But you really need to familiarize yourself with JavaScript's prototypal object model. Otherwise you're just programming by coincidence - without really knowing what the code does.

  • Don't use the with statement. It makes code hard to read and reason about. It's one of the grand mistakes in JavaScript language. Avoid it. In your case, just prefix the instance variables/functions with this. - it's not so long to write.

  • It's very unusual to place the opening { on a separate line in JavaScript. This kind of indentation style is rarely seen. I suggest you adopt the common style of placing { on the same line as if etc.

  • The common variable naming convention in JavaScript is to use camelCase not under_scores.

  • Your is_whitespace function could be much more easily implemented with just a simple regular expression:

    isWhitespace: function(s) {
        return /^\s*$/.test(s);
    }
    

And don't take my word for it. Run your JavaScript through an analyzer like JSHint - it will warn you about these and other issues.

PS. I think overall your code is very nicely and cleanly written. It just falls short on looking and feeling like JavaScript.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the comments and link to JSHint (all new to me). Re new function(), I use that intentionally to create a logical namespace, which is a single object -- no need for prototype. I avoided the object literal notation for three reasons, mostly emotional: (1) the commas instead of semicolons, (2) the thought of n-hundred lines long literal (even if the same could be said for the function body), and (3) that, having been away from Javascript for 10 or more years I'm unsure if the functions introduced in a literal can refer to each other or to other data items there? \$\endgroup\$ Commented Apr 4, 2014 at 22:30
  • \$\begingroup\$ under_scores is called snake_case afaik ;) \$\endgroup\$
    – Vogel612
    Commented Apr 7, 2014 at 12:20
3
\$\begingroup\$

From a once over:

  • Please use lowerCamelCase -> is_whitespace_character -> isWhiteSpaceCharacter, that is the standard for JavaScript
  • Considering \n and r as whitespace characters while technically legit seems old skool
  • It does not make sense to have a function to detect white space in a string, and a function to detect a single white space character, it should be 1 function. A character is a string of length 1 after all
  • Consider replacing s.substr( i, 1 ) with s[i], it is more succinct
  • Consider using better variables, anything outside of Spartan convention ( i(nteger), s(tring), o(bject), e(vent), c(har) ) should have a proper name. So var n = s.length is no good
  • Do not use with in JavaScript, most developers will not know how it works, and most developers create hard to solve bugs with with
  • I think there are succinct regex expression to check for a string being whitespace
  • Consider using .toString(16) in hex_from_int, it will cut 90% of that function
  • In local_file_url, it does not make sense to create a var and then return that variable on the next line, you should return immediately
\$\endgroup\$
5
  • \$\begingroup\$ Thanks for you comments. Regarding formatting conventions, I hear. THanks. Regarding e.g. \r as whitespace being "old skool", well if correctness is old skool then that's my thing. The code would not work correctly with a more limited whitespace notion. Re the whitespace-for-character and whitespace-for-general string, that makes eminent sense: it's far better to have separate functions than a single function that delegates to two different internal implementations (there's no point in such delegation). However, when implement as a regex there's no such consideration, but also less control. \$\endgroup\$ Commented Apr 7, 2014 at 12:23
  • \$\begingroup\$ Re string indexing, thanks, I don't know where I got the idea that JavaScript didn't support it. Of course it does. Re spartan variable names, no it's not a good idea to give e.g. a loop control variable a more descriptive name than i. Re "most developers will not know how [the with statement]` works", I have tried, many years ago, to code and design for the lowest common denominator of programmers, due to earlier arguments from those below the median. And in my experience that's not only futile but directly counter-productive, so, don't do as I did. But sure, general use of with = Evil. \$\endgroup\$ Commented Apr 7, 2014 at 12:29
  • \$\begingroup\$ Re .toString(16) for hex representation, I did consider it, and no it does not cut 90% of that function. In particular you need to handle negative numbers correctly. Re "create a var and then return that variable on the next line" as meaningless, yes it might look that way. That's a debugging hook. It was used and left as-was. ;-) IME that generally avoids doing the same work repeatedly. So, I beg to differ eith the "you should return immediately". But all in all, thanks a bunch for the observations and advice. Very through. Thanks. :-) \$\endgroup\$ Commented Apr 7, 2014 at 12:33
  • \$\begingroup\$ Re: .toString(16), it does handle negative numbers. Also there is no need to create a var as a debugging hook ? You can simply put the breakpoint on the return statement and evaluate what you are about to return ? \$\endgroup\$
    – konijn
    Commented Apr 7, 2014 at 12:36
  • \$\begingroup\$ I found that .toString(16) handles negative numbers by adding a minus sign in front. In contrast, the desired hex representation (for this code at least) is hex representation of the bitpattern. After all that's the point of hex. Re "simply put a breakpoint", I wasn't debugging with a debugger. ;-) I guess I have a lot to learn about JavaScript debugging, but what I found was that Microsoft has discontinued their old JScript debugger, and replaced with something in Office, and that even though the HTA code is executed by the IE engine I was unable to use IE debugging facilities. But, learning. \$\endgroup\$ Commented Apr 7, 2014 at 12:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.