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.

Sign up for GitHub

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

wip: feat: Make foam respect the .gitignore file #1413

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

wip: feat: Make foam respect the .gitignore file #1413

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74dc6c1
Add support for .gitignore file parsing to exclude patterns
TieWay59 Oct 24, 2024
68d2d37
Refactor .gitignore file handling to use Uri.joinPath for improved pa…
TieWay59 Nov 20, 2024
cf9a173
wip: not working try
TieWay59 Nov 26, 2024
bd3051a
Add file system watcher for .gitignore changes to prompt reload
TieWay59 Nov 28, 2024
ed3b0ae
Enhance .gitignore handling by reading patterns from all .gitignore f…
TieWay59 Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/foam-vscode/src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function activate(context: ExtensionContext) {
exposeLogger(context, logger);

try {
Logger.info('Starting Foam');
Logger.info('[wtw] Starting Foam');

if (workspace.workspaceFolders === undefined) {
Logger.info('No workspace open. Foam will not start');
Expand Down Expand Up @@ -100,6 +100,13 @@ export async function activate(context: ExtensionContext) {
'Foam: Reload the window to use the updated settings'
);
}
}),
// 2024-11-25 21:22:48 TODO wip not working as expected
workspace.createFileSystemWatcher('.gitignore').onDidChange(e => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to live inside context.subscriptions so that it's properly cleaned up afterwards

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true! The issue is that the FileSystemWatcher may be disposed if it's not added to context.subscriptions. problem solved.

Logger.info(`[wtw] File changed: ${e.fsPath}`);
window.showInformationMessage(
'Foam: Reload the window to use the updated .gitignore settings'
);
TieWay59 marked this conversation as resolved.
Show resolved Hide resolved
})
);

Expand Down
27 changes: 26 additions & 1 deletion packages/foam-vscode/src/services/editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isEmpty } from 'lodash';
import { isEmpty, map, compact, filter, split, startsWith } from 'lodash';
import {
EndOfLine,
FileType,
Expand All @@ -25,6 +25,8 @@ import {
IDataStore,
IMatcher,
} from '../core/services/datastore';
import * as path from 'path';
import { Logger } from '../core/utils/log';

interface SelectionInfo {
document: TextDocument;
Expand Down Expand Up @@ -201,6 +203,29 @@ export async function createMatcherAndDataStore(excludes: string[]): Promise<{
const excludePatterns = new Map<string, string[]>();
workspace.workspaceFolders.forEach(f => excludePatterns.set(f.name, []));

Logger.info('[wtw] Excluded patterns from settings: ' + excludes);
// Read .gitignore files and add patterns to excludePatterns
for (const folder of workspace.workspaceFolders) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this block of code should live here, I would have it together with the rest of the configuration, in order to populate the excludePatterns variable, which is then fed into this fn.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rework it later, can you hint me more about rest of the configuration ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry what do you mean? My understanding is that your blocker is only the notification to the user not working (and unfortunately I can't understand why that's the case) - am I missing something else?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes, I was so blocked in the dispose problem, so I didn't give it actual implementation and everything else is still in the experiment stage. now I'm about to do it. I'll change a lot design in the next push, so I guess we can ignore this outdated change for now.

const gitignoreUri = Uri.joinPath(folder.uri, '.gitignore');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path needs to be the same as the file(s) you are watching for changes. (this should be easy to do if you have both at the top level).
I would expect something like this in extension.ts:

  1. compute .gitignore path(s)
  2. extract exclude patterns from those
  3. merge them with the regular configuration settings and use them in this fn
  4. monitor the paths computed

try {
await workspace.fs.stat(gitignoreUri); // Check if the file exists
const gitignoreContent = await workspace.fs.readFile(gitignoreUri); // Read the file content
const patterns = map(
filter(
split(Buffer.from(gitignoreContent).toString('utf-8'), '\n'),
line => line && !startsWith(line, '#')
),
line => line.trim()
);
excludePatterns.get(folder.name).push(...compact(patterns));

Logger.info(`Excluded patterns from ${gitignoreUri.path}: ${patterns}`);
} catch (error) {
// .gitignore file does not exist, continue
Logger.error(`Error reading .gitignore file: ${error}`);
}
}

for (const exclude of excludes) {
const tokens = exclude.split('/');
const matchesFolder = workspace.workspaceFolders.find(
Expand Down