SketchUp / ruby-api-docs

SketchUp Ruby API Documentation
18 stars 8 forks source link

UI::HtmlDialog constructor #12

Closed DanRathbun closed 6 years ago

DanRathbun commented 7 years ago

UI::HtmlDialog constructor

  1. lists under initialize() instead of new() class constructor method.

  2. Example a. uses frivolous literal hash. Ruby has always collected key / value pairs into a hash argument automatically. b. Since this class can only be used with Ruby 2.2, it would make better sense to show named arguments which ruby will collect into a hash argument automatically.

    dialog = UI::HtmlDialog.new(
    dialog_title: "Dialog Example",
    preferences_key: "com.sample.plugin",
    scrollable: true,
    resizable:  true,
    width:  600,
    height: 400,
    left:   100,
    top:    100,
    min_width:    50,
    min_height:   50,
    max_width:  1000,
    max_height: 1000,
    style: UI::HtmlDialog::STYLE_DIALOG
    )
DanRathbun commented 7 years ago

Also, the use of Apple UTIs in API examples, is confusing to people who don't use them or know what they are for, (especially MS Windows people.)

The preferences key is used on Windows as a registry key name. (I wouldn't use periods in any of my registry key names.) Better to use the author's toplevel module name, plugin name and function joined with an underscore.

preferences_key: "ACME_RoadRunnerKiller_Options",

This reminds me of a long standing omission in the API docs.

That they do not inform coders that their UI::HtmlDialog and UI::WebDialog preference keys, as well as Sketchup::AttributeDictionary names, are saved in a shared environment, and need to be qualified just as their modules need to be.

thomthom commented 7 years ago

The intialize vs new is due to new being a default Ruby class method that call initialize. The various documentation tools handle this differently - I think.

YARD tend to show initialize: http://www.rubydoc.info/github/thomthom/SKUI/SKUI/Window#initialize-instance_method

Now, in the SketchUp API docs you will still see usages of new in our docs, because some classes uses an older way of allocate memory - which required taking over new.

So there is an inconsistency there. Not sure what best fix is here, short of overriding how YARD deal with initialize vs new.

The other suggestions are easier to handle though. Would be nice to get into our next pass.

DanRathbun commented 7 years ago

Well, when I made my YARD docs from Ruby stubs so it came out different. The API classes only came out listing new() as the constructor, if I explicitly added it as a class method. Otherwise there was no constructor at all, and no initialize() method listed.

It just looks weird. I thought YARD had a :nodoc: switch. Maybe a line beginning #-- to switch doc'ing off, and another beginning #++ to switch doc'ing back on?

thomthom commented 7 years ago

YARD uses tags to control visibility. You can filter by any tag. The most direct replacement of :nodoc: in YARD would be @private.

The API classes only came out listing new() as the constructor, if I explicitly added it as a class method. Otherwise there was no constructor at all, and no initialize() method listed.

I'm not sure if I follow what you are saying here. Got some example, screenshots?

Maybe a line beginning #-- to switch doc'ing off, and another beginning #++ to switch doc'ing back on?

I don't understand this one either? Turning documentation on/off?

thomthom commented 7 years ago

Well, when I made my YARD docs from Ruby stubs so it came out different.

@DanRathbun - you have a script that generate stubs from YARD? Is that something you'd be willing to share?

DanRathbun commented 7 years ago

you have a script that generate stubs from YARD?

No. (I said the opposite.)

So there is an inconsistency there. Not sure what best fix is here, short of overriding how YARD deal with initializevs new.

As mentioned in another thread:

YARD can generate method documentation from directive comments only (without stubs.)

See the directives in the YARD documentation:

DanRathbun commented 6 years ago

Migrated to stubs repo. CLOSING in this repo.