Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create initial pre-commit configuration #165

Merged
merged 2 commits into from
Sep 15, 2024

Conversation

stdedos
Copy link
Contributor

@stdedos stdedos commented Apr 2, 2024

Create initial pre-commit configuration

The commit introduces a basic pre-commit configuration
with hooks for auto-fixing common code quality issues:

  • Ensure files end with a newline (end-of-file-fixer),
  • Standardizing line endings to LF (mixed-line-ending), and
  • Removing trailing whitespace (trailing-whitespace).

Use a home-brewed pre-commit CI action, because the organization
cannot install the necessary pre-commit GitHub application.

Draw inspiration from:

Signed-off-by: Stavros Ntentos [email protected]

@stdedos
Copy link
Contributor Author

stdedos commented Apr 2, 2024

@isaak654 #111 (comment)

Here is my proposal. I hope upstream (i.e., you, in plural) is okay with installing the https://pre-commit.ci/ app.
There are other ways also, but less powerful.

Please familiarize yourself with the (short) website/demo/docs https://pre-commit.ci/.

It may be possible to see the results of my MR, after you install the pre-commit.ci in the repository, on this same MR.

@isaak654
Copy link
Collaborator

isaak654 commented Apr 4, 2024

Here is my proposal. I hope upstream (i.e., you, in plural) is okay with installing the https://pre-commit.ci/ app.

There are doubts in this regard:

xanatosdavid
well it says free for open source repositories so i guess we can install it for free
xanatosdavid
hmm... it says 0€ but still wants billing informations
xanatosdavid
woukd that tool help so much?

If you agree to include an email address in your GitHub profile, I can invite you on Slack to discuss with the repository owner.

@stdedos
Copy link
Contributor Author

stdedos commented Apr 7, 2024

@isaak654 I have tried reaching you in private on Discord

@stdedos stdedos requested a review from isaak654 May 2, 2024 09:10
@isaak654 isaak654 requested a review from stephtr July 31, 2024 11:43
@stdedos stdedos force-pushed the feat/add-pre-commit branch 7 times, most recently from 63ceaf9 to e694b53 Compare August 1, 2024 21:27
@stdedos
Copy link
Contributor Author

stdedos commented Aug 1, 2024

Action would've worked correctly "locally" (on the fork), but I have created a workflow issue (https://github.com/stdedos/sandboxie-docs/actions/runs/10206030628/job/28238064040#step:5:18), and would've worked for this repo, but GH does not allow "privilege escalation" (https://github.com/sandboxie-plus/sandboxie-docs/actions/runs/10206030965/job/28238065287?pr=165#step:1:17 vs https://github.com/stdedos/sandboxie-docs/actions/runs/10206030628/job/28238064040#step:1:17)

@stdedos stdedos force-pushed the feat/add-pre-commit branch from e694b53 to cf5b316 Compare August 1, 2024 21:30
@stdedos
Copy link
Contributor Author

stdedos commented Aug 1, 2024

The "push from the PR" needs to be improved.

... but, if we switched

  push:
    branches:
     - main
  pull_request:
    branches:
     - main

to

  push:
  pull_request:
    branches:
     - main

then, every push (including forks!) would fix this for us, like what has happened here ^^

(or altogether to

  push:
  pull_request:

I don't see "why not" enforce those basic rules to everything 😕)

@isaak654
Copy link
Collaborator

isaak654 commented Aug 2, 2024

@QZLin Will we have a secondary branch with the merging of #167?

@QZLin
Copy link
Contributor

QZLin commented Aug 2, 2024

@QZLin Will we have a secondary branch with the merging of #167?

If you mean the gh-pages branch, yes. publish.yml will call mkdocs gh-deploy, and mkdocs will create a gh-pages branch and push the built site into it.

@stdedos
Copy link
Contributor Author

stdedos commented Aug 2, 2024

@isaak654 I saw

The last option is fine with me, as we don't keep any old branch here.

So, are we good with this? Should I make my proposed changes?

@isaak654
Copy link
Collaborator

isaak654 commented Aug 2, 2024

That comment did not consider #167, let's proceed with:

  push:
  pull_request:
    branches:
     - main

@stdedos
Copy link
Contributor Author

stdedos commented Aug 2, 2024

@isaak654
Copy link
Collaborator

isaak654 commented Aug 2, 2024

@stdedos
Copy link
Contributor Author

stdedos commented Aug 4, 2024

I dropped schedule.cron and workflow_dispatch from the latest change; I haven't seen a need for these to exist yet

@stdedos stdedos force-pushed the feat/add-pre-commit branch 2 times, most recently from 7a83208 to 537048d Compare August 4, 2024 07:56
@stdedos
Copy link
Contributor Author

stdedos commented Aug 4, 2024

I would still apply

- id: mixed-line-ending
  args:
  - --fix=lf

This PR will bring "a lot of changes" - might as well bring the changes we want

@stdedos
Copy link
Contributor Author

stdedos commented Aug 4, 2024

Also statements like

      exclude: '^(Media/.*)'

are useless. Idk if you have any counter-examples, but at this state - they don't do anything for this repository.

Let's "fix them" when we have actual examples?

@stdedos stdedos force-pushed the feat/add-pre-commit branch from b715a2d to f8c4ddd Compare August 4, 2024 08:17
@stdedos
Copy link
Contributor Author

stdedos commented Aug 6, 2024

... fixed the conflicts altogether.

I am not sure how you did that. You are still on top of 178a7f4 (8e2f75a)

@stdedos stdedos force-pushed the feat/add-pre-commit branch 4 times, most recently from f3fe9d6 to 371af31 Compare August 7, 2024 07:26
@isaak654
Copy link
Collaborator

I am not sure how you did that. You are still on top of 178a7f4 (8e2f75a)

Before amending the initial commit, I've used git reset HEAD~1 to undo the auto-fixes commit.

About @offhub's report, I think we should check all .md files in 487f678 in order to find all lines with two trailing spaces.

FAQVirus.md (lines 47-49) has the same regression of InjectDll.md (lines 21-26), so any help to reduce the effort in manually examining the files would be useful, including any script or command that could help us list the files to fix.

@stdedos
Copy link
Contributor Author

stdedos commented Aug 19, 2024

I am not sure how you did that. You are still on top of 178a7f4 (8e2f75a)

Before amending the initial commit, I've used git reset HEAD~1 to undo the auto-fixes commit.

About @offhub's report, I think we should check all .md files in 487f678 in order to find all lines with two trailing spaces.

FAQVirus.md (lines 47-49) has the same regression of InjectDll.md (lines 21-26), so any help to reduce the effort in manually examining the files would be useful, including any script or command that could help us list the files to fix.

Idk why this needs to be more complicated than it needs to be 😅

$ ug -C2 '  $'
docs/Content/ApplicationsSettings.md
    51-	[Sandboxie Control](SandboxieControl.md) > [Sandbox Settings](SandboxSettings.md) > Applications > Email Reader
    52-	
    53:	![](../Media/EmailReaderSettings.png)  
    54-	
    55-	This settings page offers quick configuration for the following email programs:

docs/Content/DeleteSandbox.md
     1-	# Delete Sandbox
     2-	
     3:	[Sandboxie Control](SandboxieControl.md) > [Sandbox Menu](SandboxMenu.md) > Delete Contents  
     4-	[Sandboxie Control](SandboxieControl.md) > [Tray Icon Menu](TrayIconMenu.md) > Delete Contents
     5-	

docs/Content/FAQVirus.md
    45-	### Q. Will viruses remain in the sandbox after I close all programs in the sandbox?
    46-	
    47:	A. Yes and no:  
    48:	1\. No, if your sandbox is set to [automatically](DeleteSettings.md#invocation) delete;  
    49:	2\. Yes, in the configuration, but only until you [manually](DeleteSandbox.md) delete the contents of the sandbox.  
    50-	It is important to note that a virus file in the sandbox is just that -- _a file_, not much different from your average text file. Unless you move the file out of the sandbox and invoke it, there
    51-	
--
    56-	### Q. Why does my anti-virus detect a virus in the _System Volume Information_ folder?
    57-	
    58:	A. The System Restore component in Windows collects various files into the _System Volume Information_ when they are deleted. While the intention is to protect your system, sometimes System Resto
    59-	Note that this will not occur if you securely wipe the contents of the sandbox (see previous question).
    60-	

docs/Content/ForceProcess.md
    15-	   ForceProcess=outlook.exe
    16-	   ForceProcess=cl?cke?.exe
    17:	   
    18-	```
    19-	

docs/Content/InjectDll.md
    19-	The order of DLLs loaded into the sandboxed program is thus:
    20-	
    21:	Ntdll.dll  
    22:	KernelBase.dll (only on Windows 7)  
    23:	Kernel32.dll  
    24:	SbieDll.dll (on 64-bit Windows, this can be either the 64-bit SbieDll or the 32-bit SbieDll)  
    25:	_InjectDlls_ (loaded in the order specified in Sandboxie.ini)  
    26:	Optionally, ShimEng (or AppHelp on Windows 7) and related DLLs  
    27-	All [statically-linked](https://msdn.microsoft.com/en-us/library/ms684184(VS.85).aspx) DLLs
    28-	

docs/Content/OpenProtectedStorage.md
    12-	Indicates that programs running in the DefaultBox sandbox will update the global system [Protected Storage](ProtectedStorage.md), rather than a sandboxed instance of it.
    13-	
    14:	Related [Sandboxie Control](SandboxieControl.md) setting: _Save outside sandbox: History of search strings and invoked commands_  
    15-	in [Sandbox Settings > Applications > Web Browser](ApplicationsSettings.md#web-browser)

docs/Content/ProtectHostImages.md
     1-	# Protect Host Images 
     2-	
     3:	_ProtectHostImages_ is a sandbox setting in [Sandboxie Ini](SandboxieIni.md) available since v1.9.0 / 5.64.0. This setting can be enabled to prevent processes located outside the sandbox from loa
     4-	
     5-	```

docs/Content/QuickRecovery.md
     1-	# Quick Recovery
     2-	
     3:	[Sandboxie Control](SandboxieControl.md) > [Sandbox Menu](SandboxMenu.md) > Quick Recovery  
     4-	[Sandboxie Control](SandboxieControl.md) > [Tray Icon Menu](TrayIconMenu.md) > Quick Recovery
     5-	

docs/Content/ResourceAccessSettings.md
    11-	Examples where exceptions are convenient or necessary:
    12-	
    13:	*   Allow direct access to some specific folder. For example, let the Web browser place downloads directly in a _Downloads_ folder.  
    14-	    See the [File Access](ResourceAccessSettings.md#file-access) category below.
    15-	*   A program may need access to some resource for correct operation. If the program is known and trusted, it is reasonable to make such an exception. See [Known Conflicts](KnownConflicts.md) for

docs/Content/SandboxMenu.md
    14-	
    15-	
    16:	*   The _Web Browser_ command starts the system (default) Web browser.  
    17-	    (Note: If the wrong program starts, see [Frequently Asked Questions](FrequentlyAskedQuestions.md#why-does-the-wrong-program-start-when-i-run-my-default-web-browser-sandboxed) to fix this.)
    18-	
--
    62-	The special variable **%SANDBOX%** is replaced by the name of the sandbox.
    63-	
    64:	The special variable **%USER%** is replaced by the name of whichever user account (or logon) is using that sandbox. Note that a sandbox created in one user account is visible and can be used by o
    65-	However, if the container folder includes the **%USER%** special variable, then the user accounts don't actually share the same sandbox. Each account has a separate instance of the sandbox.
    66-	

docs/Content/TestEmailConfiguration.md
     7-	After completing the email configuration, you may want to test it to make sure that new emails will not be lost when you delete the sandbox. To do that, follow these steps:
     8-	
     9:	*   Disable Internet access in the sandbox. This is a precaution measure, to make sure that your sandboxed email program cannot retrieve new mail messages before you confirm the configuration is 
    10-	    Open [Sandbox Settings > Restrictions > Internet Access](RestrictionsSettings.md#internet-access), then click _Block All Programs_, and finally click _OK_.
    11-	
--
    25-	If the email message that you created in a sandboxed instance of your email program is also accessible in the normal (unsandboxed) instance, even after the sandbox has been deleted, then the conf
    26-	
    27:	*   When done, re-enable Internet access in the sandbox:  
    28-	    Open [Sandbox Settings > Restrictions > Internet Access](RestrictionsSettings.md#internet-access), then click _Remove_ (to remove the restriction), and finally click _OK_.
    29-	

docs/Content/TrayIconMenu.md
    17-	One or more sub-menus appear for each sandbox defined. The default configuration includes only one sandbox named _DefaultBox_, but more can be added using the [Sandbox Menu](SandboxMenu.md). Each
    18-	
    19:	*   The _Run Web Browser_ command starts the system (default) Web browser.  
    20:	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Run Sandboxed -> Web Browser.  
    21-	    (Note: If the wrong program starts, see [Frequently Asked Questions](FrequentlyAskedQuestions.md#why-does-the-wrong-program-start-when-i-run-my-default-web-browser-sandboxed) to fix this.)
    22-	
    23:	*   The _Run Email Reader_ command starts the system (default) email reader.  
    24-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Run Sandboxed -> Email Reader.
    25-	
    26:	*   The _Run Any Program_ command displays the Run Any Program dialog box which is similar to the standard Windows _Run..._ dialog box. It can be used to start programs, open documents, and brows
    27-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Run Sandboxed -> Any Program.
    28-	
    29:	*   The _Run From Start Menu_ command displays the Sandboxie Start menu, similar to the standard Windows Start menu. It can be used to start programs and other shortcuts that appear in the start 
    30-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Run Sandboxed -> From Start Menu.
    31-	
    32:	*   The _Run Windows Explorer_ command starts a sandboxed instance of the Windows Explorer. It can be used to navigate folders and start programs, all under the supervision of Sandboxie.  
    33-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Run Sandboxed -> Windows Explorer.
    34-	
    35:	*   The _Terminate Programs_ command stops all programs running in the sandbox.  
    36-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Terminate Running Programs.
    37-	
    38:	*   The _Quick Recovery_ command shows the [Quick Recovery](QuickRecovery.md) window.  
    39-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Quick Recovery.
    40-	
    41:	*   The _Delete Contents_ command shows the [Delete Sandbox](DeleteSandbox.md) window.  
    42-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Delete Contents.
    43-	
    44:	*   The _Explore Contents_ command opens an _unsandboxed_ folder view for the contents of the sandbox _outside the supervision of Sandboxie_. If possible, use the [Files And Folders View](FilesAn
    45-	    Same as [Sandbox Menu](SandboxMenu.md) -> _(sandbox)_ -> Explore Contents.
    46-	
--
    49-	### Terminate All Programs
    50-	
    51:	The _Terminate All Programs_ command stops all programs running in all sandboxes.  
    52-	Same as [File Menu](FileMenu.md) -> Terminate All Programs.
    53-	
--
    58-	### Disable Forced Programs
    59-	
    60:	The _Disable Forced Programs_ toggle command temporarily disables and re-enables forced sandboxing. See the associated command in the [File Menu](FileMenu.md). Note that unlike the File Menu comm
    61-	Same as [File Menu](FileMenu.md) -> Disable Forced Programs.
    62-	
--
    67-	### Run As UAC Administrator
    68-	
    69:	The _Run As UAC Administrator_ (not shown in the picture; see [File Menu](FileMenu.md)) toggle command tells Sandboxie to ask for elevation to Administrative privileges before starting any progra
    70-	Same as [File Menu](FileMenu.md) -> Run As UAC Administrator.
    71-	
--
    76-	### Exit
    77-	
    78:	The _Exit_ command quits [Sandboxie Control](SandboxieControl.md). Note that merely closing the window (or selecting the _Hide Window_ command) _does not_ quit Sandboxie Control.  
    79-	Same as [File Menu](FileMenu.md) -> Exit.
    80-	

We just need to fix the files "accordingly" (I guess this is for you @isaak654, being the docs gatekeeper), and then verify that the style is the same (or similar).

The latter can fall again on you, or, if you decided to host documentation e.g. to ReadTheDocs - you could use https://docs.readthedocs.io/en/stable/pull-requests.html?

@isaak654
Copy link
Collaborator

isaak654 commented Sep 8, 2024

See https://github.com/sandboxie-plus/sandboxie-docs/compare/fixes for the ongoing fixes.

  • Please rerun ug -C2 ' $' in the Content folder there*
    *I have left on purpose a few trailing whitespaces to test the pre-commit bot afterwards.

Other things you might need to check:

  • Fix the conflicting file in this PR
  • Did you previously run ug -C2 ' $' in the PlusContent folder?

@stdedos
Copy link
Contributor Author

stdedos commented Sep 9, 2024

See fixes (compare) for the ongoing fixes.

  • Please rerun ug -C2 ' $' in the Content folder there*
    *I have left on purpose a few trailing whitespaces to test the pre-commit bot afterwards.

image

ug is "just a fancy grep". ofc I have no problem checking it from my side, but you can verify it yourself either via grep - or from pre-commit itself.
(Indeed you left only two files with two trailing whitespaces, but there are some more with one trailing whitespace)

Other things you might need to check:

  • Fix the conflicting file in this PR

Done ✅

  • Did you previously run ug -C2 ' $' in the PlusContent folder?

Especially in this case, I run commands from the top-level - as pre-commit does 😅

@stdedos stdedos force-pushed the feat/add-pre-commit branch from 371af31 to e8d2a6a Compare September 9, 2024 14:43
@stdedos
Copy link
Contributor Author

stdedos commented Sep 9, 2024

... but I guess anyway I needed to wait for https://github.com/sandboxie-plus/sandboxie-docs/compare/fixes to be merged before rebasing this one :/

@isaak654 isaak654 force-pushed the feat/add-pre-commit branch 8 times, most recently from 1d9150f to 2327ce2 Compare September 15, 2024 20:21
The commit introduces a basic `pre-commit` configuration
with hooks for auto-fixing common code quality issues:

* Ensure files end with a newline (`end-of-file-fixer`),
* Standardizing line endings to LF (`mixed-line-ending`), and
* Removing trailing whitespace (`trailing-whitespace`).

Use a home-brewed pre-commit CI action, because the organization
cannot install the necessary pre-commit GitHub application.

Draw inspiration from:
* https://github.com/isaak654/sandboxie-docs/blob/6169c206fa9978641dafddf34303404604beaf33/.pre-commit-config.yaml
* https://github.com/isaak654/sandboxie-docs/blob/84751989d8fdee890520f116ea9e2ab0ff9f9b53/.github/workflows/action.yml#L59-L87

Signed-off-by: Stavros Ntentos <[email protected]>


skip-checks: true
@isaak654 isaak654 force-pushed the feat/add-pre-commit branch 2 times, most recently from bf9ccad to 430c444 Compare September 15, 2024 21:24
@isaak654 isaak654 merged commit e78c57e into sandboxie-plus:main Sep 15, 2024
@isaak654
Copy link
Collaborator

New commit reference in sandboxie-plus:main: bce1b27
[pre-commit.ci] reference in sandboxie-plus:main: 7145afc

@isaak654 isaak654 removed the request for review from stephtr September 15, 2024 21:44
@sandboxie-plus sandboxie-plus deleted a comment from stdedos Sep 15, 2024
@sandboxie-plus sandboxie-plus deleted a comment from stdedos Sep 15, 2024
@sandboxie-plus sandboxie-plus locked and limited conversation to collaborators Sep 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants