secure-77 / Perlite

A web-based markdown viewer optimized for Obsidian
https://perlite.secure77.de/
MIT License
968 stars 81 forks source link

Update composer.json, move lib files to src folder (renewed) #102

Closed selfiens closed 9 months ago

selfiens commented 11 months ago

Related to #67, #101

This PR introduces a Perlite namespace in composer.json and maps it to the newly created /perlite/.src folder. This enables class autoloading in accordance with PSR-4 standards.

The code changes have been minimized compared to #101. However, one line in helper.php was modified during local testing to accommodate OS-independent handling of the temp folder.

I recognize that using .src as a namespace root is unconventional. While src is more commonly adopted, there's a potential risk of it colliding with a user's vault name (and the vendor folder too). The prefix . in src is intended as a temporary measure until the project undergoes a folder structure reorganization. I welcome any suggestions or feedback.

secure-77 commented 11 months ago

Thank you again for this PR,

On more thing comes to my mind: is it possible to specify the path for the autoloader on the build? In my current .gitignore I have an entry for the vendor folder to avoid pushing lots of unnecessary files to this repo, but I want that a clone of this repo will also work without building it via composer, also I need to take care to copy only necessary files to the docker image on the build process.

So my question is, if we introduce a namespace and a new sub folder (vendor) for the autoloader.php can we change the structure that we only need one new sub folder for autoloader.php, PerliteParsedown.php, Parsedown.php etc.

So I would recommend to either move also the PerliteParsedown.php to the vendor folder and I change the .gitingore to exclude everything non needed from the vendor folder or we go the other way to have a new (src) folder and move the autoloader.php PerliteParsedown and Parsedown.php to this folder.

You know what I mean?

selfiens commented 11 months ago

Thank you for taking the time to review my PR :)

Your concerns are very thoughtful. I completely support your ideas. End users should be able to git clone without any extra steps.

Below are brief pros and cons of including/excluding the vendor folder.

Criteria Including vendor Excluding vendor
Newcomer Experience Easier setup for beginners. Requiring Composer can be a big huddle.
Dev Experience Acceptable, following the nature of the project. Common practice. But some devs are not familiar with Composer.
Consistency Everyone uses the exact same set of libraries. Possible variation in library versions if not managed properly.
Offline Development Possible. Requires internet connection initially to fetch dependencies.
Repository Size Increased size due to all dependency files. Only your project's direct code.
Security Risk if malicious code gets into vendor and goes unnoticed. vendor is managed by Composer, still need to trust 3rd party.

Based on my understanding, your question is about having one folder to store both 1st and 3rd party class files.

1) Hosting 1st + 3rd party class files in a single folder.

Here are my opinions:

My view on a more common folder structure is:

/perlite
    /public - web root
        /favicon.ico
        /.js
        /.styles
        /index.php
        /(user vaults)
    /src - project's unique classes
    /vendor
    /composer.json

Developers familiar with Composer or those coming from popular frameworks will feel comfortable with this structure.

The integration of Composer entails significant decisions. However, I believe in the value of integrating Composer for the future of this project. I'll answer your questions to the best of my knowledge to help you make the right decision. Any additional questions, new ideas, or entirely different approaches are welcome :)

secure-77 commented 11 months ago

Thank you for the extensive information! I think I will follow your suggestions. Unfortunately, this means I need to exclude the JS dependencies again from the composer and either update them manually, use CDNs or use a second composer file and a bash script. However, if there is a best practice in development, I will try to follow it :) I will try to include this in the next release.

secure-77 commented 9 months ago

I decided to use a separate php composer file for the js assets. This way I can include the vendor folder and use the php autoload script. Release is now online in 1.5.7.

Thanks again for the contribution!