ludorival / vscode-preview-jsdoc

VSCode extension to automatically preview the generated jsdoc
MIT License
6 stars 1 forks source link

preview jsdoc doubles doc entries #25

Closed LukasWillin closed 5 years ago

LukasWillin commented 5 years ago

Hi Ludovic

Lukas Again... :P

So following issue: In my jsdocconf.json file I have the root source code folder set to be included so that the plugin would always create docs for the whole project. But sometimes it would double some entries or complete classes e.g.: grafik So this is already weird but even more interesting is, that it wouldnt do it for all docs and not even within the same class consistently. I first thought that this is an issue of jsdoc since I had a lot of other problems which were related to jsdoc directly. I then tried just out of curiosity to generate the docs from the commandline directly via jsdoc and viola the cloned entries are gone! grafik So I then looked more closely which classes are affected when this happens and it turns out that it only affects those files within the same folder (& subfolders).

So I guess since the preview jsdoc runs jsdoc from the edited files parent folder it processes the same files twice. (Which is a bommer!) At this point I dont know what you could do about this. I guess if you would use the project root it would just always double everything? Im not sure though. Here are all my settings and tools used:

Version Info

tool version
node v11.0.0
npm 6.4.1
jsdoc JSDoc 3.5.5 (Thu, 14 Sep 2017 02:51:54 GMT)
vscode-preview-jsdoc 2.0.5

jsdocconf.json

{
  "plugins": [ ],
  "recurseDepth": 20,
  "source": {
    "exclude": [
      "node_modules",
      "dist",
      "lib",
      "spec",
      "jasmine",
      "wiki",
      "src/babel/"
    ],
    "includePattern": ".+\\.js(doc|x)?$",
    "excludePattern": "(^|\\/|\\\\)_",
    "include": [
      "src/tell"
    ]
  },
  "sourceType": "module",
  "tags": {
    "allowUnknownTags": true,
    "dictionaries": [
      "jsdoc",
      "closure"
    ]
  },
  "templates": {
    "cleverLinks": true,
    "monospaceLinks": false
  },
  "opts": {
    "encoding": "utf8",
    "recurse": true,
    "verbose": true,
    "destination": "wiki/www"
  }
}

Workspace Settings

"settings": {
  "previewjsdoc.confFile": "jsdocconf.json",
  "previewjsdoc.withPrivate": true,
  "previewjsdoc.output": "wiki"
}

The jsdoc command line used to generate docs manually (from project root)

jsdoc -c jsdocconf.json

Cheers

LukasWillin commented 5 years ago

Okay so yeah I didnt think of this first but when I edit a file in the root folder it doubles literally everything: grafik This is just one module and its classes but it is the same for everything else as well. Most class members are cloned to.

I dont want to remove the "include" setting in my jsdoconf.json though because then I wouldnt always have the whole project in my jsdoc.

From the output of your extension I found that you pass the parent path of the edited file. Maybe you can ommit the explicit cli source parameter to generate the docs? Or somehow make it an opt-out option.

jsdoc C:\Users\l.willin\.vscode\extensions\ludorival.preview-jsdoc-2.0.5\node_modules\jsdoc\jsdoc.js --verbose -d "c:\Projects\Tell.js\wiki\www" -c "C:/Projects/Tell.js/jsdocconf.json" -p "c:\Projects\Tell.js\src\tell"
LukasWillin commented 5 years ago

Just out of curiosity. Is there any special reason why you pass the parent folder explicitly in the commandline?

LukasWillin commented 5 years ago

Okay so I hope i didnt cross a line with this one but I looked at your plugin code and out-commented the following:

from the transpiled vscode-preview-jsdoc/src/utils.ts (utils.js)

// ...
    }
    return null;
}
exports.getSupportedExtension = getSupportedExtension;
function spawnJsdoc({ destination, root, sourceDirectory, confFile, withPrivate, tutorials }) {
    const args = [
        jsdocPath,
        '--verbose',
        '-d',
        `"${destination}"`,
        ...(confFile ? ['-c', `"${confFile}"`] : []),
        ...(withPrivate ? ['-p'] : []),
        ...(tutorials ? ['-u', `"${tutorials}"`] : []),
        // ...(sourceDirectory ? [`"${sourceDirectory}"`] : []), // πŸ”Ά
    ];
    return { spawn: cp.spawn('node', args, { shell: true, cwd: `${root}` }),
        args };
}
exports.spawnJsdoc = spawnJsdoc;
//# sourceMappingURL=utils.js.map

This did the trick for me. Its up to you but I think to me it would be better to ommit all settings which dont really extend jsdoc since this will already be handled by the jsdoc conf.json. (Like the withPrivates or the sourceDirectory)

But as always these are just my two cents. πŸ˜‹

ludorival commented 5 years ago

Hi Lukas,

Don't worry, you have well done ^^. In fact, I was thinking I was managed this kind of issue through the method

function asIncludedInSource({source, root, sources}: {
    source: string, root: string, sources: string[]}): string | undefined {
    const find = sources.map((value) => asAbsolutePath({source: value, root}))
                        .map((absolutePath) => path.relative(absolutePath, source))
                        .find((relativePath) => relativePath.startsWith('..'));
    return find ? undefined : source;
}

This methods looks at the jsdoc conf inside the include array and check if the current source directory is already included in this array. But maybe there is a case I missed. Could you please give me the

I will try to reproduce on my side.

Again thanks Lukas, your feedback (good as bad) are really appreciated πŸ˜ƒ

LukasWillin commented 5 years ago

1) The cli commands from your plugin are as follows:

This is the command it executes per default with my conf file etc. Note however that the sourceDirectory path changes often since I organize all my modules in subfolders (Which is thanks to javascript quite a pain πŸ˜‹)

jsdoc C:\Users\l.willin\.vscode\extensions\ludorival.preview-jsdoc-2.0.5\node_modules\jsdoc\jsdoc.js --verbose -d "c:\Projects\Tell.js\wiki\www" -c "C:/Projects/Tell.js/jsdocconf.json" -p "c:\Projects\Tell.js\src\tell"

This is how it looks with my dirty hot fix in the utils.js file

jsdoc C:\Users\l.willin\.vscode\extensions\ludorival.preview-jsdoc-2.0.5\node_modules\jsdoc\jsdoc.js --verbose -d "c:\Projects\Tell.js\wiki\www" -c "C:/Projects/Tell.js/jsdocconf.json" -p

2) As per the jsdoc configuration file see my first post. 3) With the settings.xml you mean the settings.json?

These are my explicit workspace settings for previewjsdoc

...
"settings": {
    "previewjsdoc.confFile": "C:/Projects/Tell.js/jsdocconf.json",
    "previewjsdoc.withPrivate": true,
    "previewjsdoc.output": "wiki"
}
...

And here the workspace default settings

 // Automatically open the browser window when saving a js or md file
  "previewjsdoc.autoOpenBrowser": true,

  // This properties is deprecated. You should use the previewjsdoc.confFile setting instead
  // JSON Configuration options. More details on http://usejsdoc.org/about-configuring-jsdoc.html
  "previewjsdoc.conf": null,

  // The Json configuration file, it can be relative from the workspace or absolute.
  //  More details on http://usejsdoc.org/about-configuring-jsdoc.html
  "previewjsdoc.confFile": "",

  // The output where jsdoc generates files. 
  // If not defined, the default output is in a cache directory.
  "previewjsdoc.output": "",

  // List of tutorials paths which will be concatenated to a single one and used instead of opts.tutorials
  "previewjsdoc.tutorials": [],

  // Include symbols marked with the @private tag in the generated documentation. By default, private symbols are not included.
  "previewjsdoc.withPrivate": false

If there is more info I can give you let me know. I guess I will tamper around a bit in the function you pointed out to me on thursday evening and see what I can gather. Tomorrow Ill be probably 100% occupied with my studies

LukasWillin commented 5 years ago

Okay so I dug around a bit: Issue seems to be that the util function path.relative as used in asIncludedInSource produces the following result based on the following folder structure and parameters:

function asIncludedInSource({ source, root, sources, onLogInfo }) {
    const find = sources.map((value) => utils_1.asAbsolutePath({ source: value, root }))
        .map((absolutePath) => {
            onLogInfo(`path.relative("${absolutePath}", "${source}") => "${path.relative(absolutePath, source)}"`);
            return path.relative(absolutePath, source);
        })
        .find((relativePath) => relativePath.startsWith('..'));
    return find ? undefined : source;
}
// output is: path.relative("c:\projects\Tell.js\src\tell", "c:\projects\Tell.js\src\tell\dependency") => "dependency"

It then checks if any of the included sources from the jsdocconf.json is a subfolder of the source directory by looking for '..' in the beginning.

In my case though the include or sources path is a parent of the source directory which is why it passes the source path anyways explicitly to the cli (because '..' is not in the path and therefor find === undefined). I do this because I always want the whole project to be processed. (Which I think would always be the desired output but that could also just be me).

So at this point I dont know how you could check for both cases since if you would you could as well just ommit the source directory unless there are actually no paths set to be included in the jsdocconf.json.

So I guess my solution would be this (tested for my use case)

function asIncludedInSource({ source, root, sources }) {
    // If there is anything set in the `include` setting always ommit passing the source
    return sources.length ? undefined : source;
}

Of course this is more a quirk of jsdoc rather than your plugin. I am really not bashing on your work ✌

LukasWillin commented 5 years ago

I tested a few more things and the base line for jsdoc is this: Whenever we have a folder included twice all the docs are duplicated for that directory. πŸ˜’ But as mentioned in the previous post this is (probably a known) quirk of jsdoc

Another example would be

...
"include": [
      "src/tell",
      "src/tell/reference-unit/parts" // Everything in this directory is duplicated
]
...
ludorival commented 5 years ago

You right, jsdoc doesn't manage well the duplication of sources.

Thank you again for your analysis, you may found a bug on the asIncludedSources.

I am a bit busy this week but feel free to contribute to this project by cloning it and suggest your pull request πŸ˜‰. I will really appreciate.

Best, Ludo

Le mer. 30 janv. 2019 Γ  14:04, Lukas Willin notifications@github.com a Γ©crit :

I tested a few more things and the base line for jsdoc is this: Whenever we have a folder included twice all the docs are duplicated for that directory. πŸ˜’ But as mentioned in the previous post this is (probably a known) quirk of jsdoc

Another example would be

... "include": [

  "src/tell",

  "src/tell/reference-unit/parts" // Everything in this directory is duplicated

]

...

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ludorival/vscode-preview-jsdoc/issues/25#issuecomment-458936122, or mute the thread https://github.com/notifications/unsubscribe-auth/AAogyqYRrAZz1avqTYs4epkdHNiG-f79ks5vIZhggaJpZM4aXE7h .

LukasWillin commented 5 years ago

Sure I'll do :) Not sure though if I manage to do it today. We'll see

LukasWillin commented 5 years ago

So I changed the outcome slightly.

My first solution was to not pass the source directory at all whenever there is something set to be include in the jsdoc.conf.json.

My new solution is to check if the source directory is a sibling (eg. a branching directory).

So lets assume the following folder structure:

src/sub1/sub1_1
src/sub2/
jsdoc.conf.json

and the jsdoc.conf.json

...
  include: [ "src/sub1" ],
...

Following would be the result when editing a file: src/sub1/sub1_1/file.js -> Dont pass source dir (subdir) src/sub1/file.js -> Dont pass source dir (same) src/file.js -> Dont pass source dir (parent) src/sub2/file.js -> Pass source dir because branching off from sub1 (aka sibling)

So unless the its not the same, a parent or a sub dir we can pass the source directory.

Tell me what you think. :)

Cheers Lukas

ludorival commented 5 years ago

Hello Lukas, Your solution sounds great. Indeed, this is not enough to check if the current directory is included in any paths of jsdoc.includes but check also if the current directory is not a parent of those paths. Maybe we can find some npm modules which help us to resolve that. I know there is the find module which is powerful to find files from a directory, but maybe we can found others otherwise we have to program this algorithm.

Do you want to take care of it? I can explain you if you want how to install the environment development

Best, Ludo

LukasWillin commented 5 years ago

Hi Ludo

Pardon me I think I dont quite understand what you mean by "it is not enough" :P I have written tests which seem to pass just fine but it could very well be I am missing something.

Can you explain where the issue lies in my solution?

Best Lukas

ludorival commented 5 years ago

Hi Lukas, maybe I was not very clear (sorry for my english by the way). I just wanted to confirm your solution and to revoke mine since mine was based on finding if the current directory is included in one of paths.

I will take a look to your first Pull Request 😁.

Best, Ludo

Le sam. 2 fΓ©vr. 2019 Γ  15:31, Lukas Willin notifications@github.com a Γ©crit :

Hi Ludo

Pardon me I think I dont quite understand what you mean by "it is not enough" :P I have written tests which seem to pass just fine but it could very well be I am missing something.

Can you explain where the issue lies in my solution?

Best Lukas

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ludorival/vscode-preview-jsdoc/issues/25#issuecomment-459969637, or mute the thread https://github.com/notifications/unsubscribe-auth/AAogylepUInSyn_vBV8xRtxYAzs1t6hgks5vJaFdgaJpZM4aXE7h .

-- Ludovic Dorival Software Engineer graduated at ENSSAT 43 rue des Chantiers 78000 VERSAILLES FRANCE Mobile : 0668188112

LukasWillin commented 5 years ago

Oh sry. I think I missread that one. I wasnt quiet sure πŸ˜ƒ

I will look into it again!

Thanks for your constructive criticism!

Best, Lukas

ludorival commented 5 years ago

I closed your issue since you fixed it πŸ˜„. The new version should we available soon. Just need to change the version and push a tag in order travis deploys itself

LukasWillin commented 5 years ago

Nice! πŸ™Œ It is a cool feeling to contribute and I sure learned a lot again. Thank you for your time and letting me do this. 😊

Best, Lukas

ludorival commented 5 years ago

You're welcome, this is the good thing from open source projects, it is a place where we can learn each others and feel like we bring something even little to the developer community 😊. Any way, your contribution is now available on the marketplace 😜.

Have a good night(?)

Thanks, Ludo

Le dim. 3 fΓ©vr. 2019 Γ  17:43, Lukas Willin notifications@github.com a Γ©crit :

Nice! πŸ™Œ It is a cool feeling to contribute and I sure learned a lot again. Thank you for your time and letting me do this. 😊

Best, Lukas

β€” You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ludorival/vscode-preview-jsdoc/issues/25#issuecomment-460067269, or mute the thread https://github.com/notifications/unsubscribe-auth/AAogygK43nsre1Bo4qLTT8-h0qccboHMks5vJxGtgaJpZM4aXE7h .

-- Ludovic Dorival Software Engineer graduated at ENSSAT 43 rue des Chantiers 78000 VERSAILLES FRANCE Mobile : 0668188112

LukasWillin commented 5 years ago

I have already DL'd it 😊 Working great!

Agreed. The open source community is great!

Not good night yet πŸ˜„ Swiss time here its 18:34 and still enough time to get things done. If it is for you then then I wish you good night as well.

We'll sure hear each other again

Wish you the best Lukas