-
Notifications
You must be signed in to change notification settings - Fork 20
π CodeRabbit Chat: Implement requested code changes #287
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| using System; | ||
| using GenHub.Core.Models.Enums; | ||
|
|
||
| namespace GenHub.Core.Models.Workspace; | ||
|
|
@@ -22,8 +23,26 @@ | |
| ContentType.Patch => 90, // Patches override base content | ||
| ContentType.GameClient => 50, // Community executables override official | ||
| ContentType.Addon => 40, // Addons (maps, etc.) | ||
| ContentType.GameInstallation => 10, // Lowest: Base game files | ||
| _ => 0, // Unknown/undefined types | ||
| ContentType.MapPack => 40, // Map collections (same tier as Addon) | ||
| ContentType.Mission => 40, // Story missions (same tier as Addon) | ||
| ContentType.Map => 40, // Individual maps (same tier as Addon) | ||
| ContentType.LanguagePack => 30, // Localization packs | ||
| ContentType.Skin => 30, // UI/visual customizations | ||
| ContentType.Video => 20, // Video content | ||
| ContentType.Replay => 20, // Replay files | ||
| ContentType.Screensaver => 20, // Screensaver files | ||
| ContentType.Executable => 20, // Standalone executables | ||
| ContentType.ModdingTool => 20, // Modding/mapping tools | ||
| ContentType.ContentBundle => 0, // Meta: collection of other content | ||
| ContentType.PublisherReferral => 0, // Meta: link to publisher content | ||
| ContentType.ContentReferral => 0, // Meta: link to specific content | ||
| ContentType.UnknownContentType => 0, // Unknown: lowest priority | ||
| ContentType.GameInstallation => 10, // Lowest physical: Base game files | ||
| _ => throw new ArgumentOutOfRangeException( | ||
|
Check warning on line 41 in GenHub/GenHub.Core/Models/Workspace/ContentTypePriority.cs
|
||
| nameof(contentType), | ||
| contentType, | ||
| $"ContentType '{contentType}' is not mapped in {nameof(ContentTypePriority)}. " + | ||
|
Check warning on line 44 in GenHub/GenHub.Core/Models/Workspace/ContentTypePriority.cs
|
||
| "Add an explicit priority entry for this type.") | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,22 @@ | |
|
|
||
|
|
||
|
|
||
| <!-- Version label button: transparent, no hover/pressed overlay --> | ||
| <Style Selector="Button.version-label"> | ||
| <Setter Property="Background" Value="Transparent" /> | ||
| <Setter Property="BorderThickness" Value="0" /> | ||
| <Setter Property="Padding" Value="0" /> | ||
| <Setter Property="Template"> | ||
| <ControlTemplate> | ||
| <ContentPresenter Content="{TemplateBinding Content}" | ||
| ContentTemplate="{TemplateBinding ContentTemplate}" | ||
| Padding="{TemplateBinding Padding}" | ||
| HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" | ||
| VerticalContentAlignment="{TemplateBinding VerticalContentAlignment}" /> | ||
| </ControlTemplate> | ||
| </Setter> | ||
| </Style> | ||
|
|
||
| <!-- Header gradient background --> | ||
| <Style Selector="Border.header-background"> | ||
| <Setter Property="Background"> | ||
|
|
@@ -212,12 +228,17 @@ | |
| <Grid Grid.Row="1"> | ||
| <ContentControl Content="{Binding CurrentTabViewModel}" /> | ||
| <!-- Version label in bottom right corner --> | ||
| <TextBlock Text="{x:Static constants:AppConstants.DisplayVersion}" | ||
| FontSize="9" | ||
| Foreground="#808080" | ||
| HorizontalAlignment="Right" | ||
| VerticalAlignment="Bottom" | ||
| Margin="0,0,10,5" /> | ||
| <Button Classes="version-label" | ||
| Command="{Binding CopyVersionToClipboardCommand}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: The XAML binds to |
||
| HorizontalAlignment="Right" | ||
| VerticalAlignment="Bottom" | ||
| Margin="0,0,10,5" | ||
| Cursor="Hand" | ||
| ToolTip.Tip="Click to copy full version (including git hash) to clipboard"> | ||
| <TextBlock Text="{x:Static constants:AppConstants.FullDisplayVersion}" | ||
| FontSize="9" | ||
| Foreground="#808080" /> | ||
| </Button> | ||
| </Grid> | ||
| </Grid> | ||
| </UserControl> | ||
| </UserControl> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -408,7 +408,7 @@ | |
|
|
||
| // Remove source.path mapping file if it exists | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifestId.Value); | ||
| var sourcePathFile = Path.Combine(contentDir, "source.path"); | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
|
Check failure on line 411 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You need to add the constant to public const string SourcePathFileName = "source.path";Additionally, Rule Used: Use dedicated constants classes instead of hardcod... (source) Prompt To Fix With AIThis is a comment left during a code review.
Path: GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
Line: 411
Comment:
**`FileTypes.SourcePathFileName` does not exist β compile error**
`FileTypes.SourcePathFileName` is referenced in three places in this file (lines 411, 507, 553), but it is never defined in the `FileTypes` constants class (`GenHub.Core/Constants/FileTypes.cs`). This will cause a build failure.
You need to add the constant to `FileTypes.cs`:
```cs
public const string SourcePathFileName = "source.path";
```
Additionally, `ContentManifestPool.cs:297` still uses the hardcoded `"source.path"` string and should also be updated to use the new constant for consistency.
**Rule Used:** Use dedicated constants classes instead of hardcod... ([source](https://app.greptile.com/review/custom-context?memory=53453b3b-b708-4856-b1b0-0cbc8bfe5330))
How can I resolve this? If you propose a fix, please make it concise. |
||
| FileOperationsService.DeleteFileIfExists(sourcePathFile); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: The constant |
||
|
|
||
| // Clean up the data directory if empty | ||
|
|
@@ -495,13 +495,18 @@ | |
|
|
||
| // Create source.path mapping if we have a valid source directory | ||
| // This allows GetContentDirectoryAsync to resolve the content location | ||
| bool contentDirCreatedByThisCall = false; | ||
| bool sourcePathWrittenByThisCall = false; | ||
|
Check warning on line 499 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| if (!string.IsNullOrWhiteSpace(sourceDirectory) && Directory.Exists(sourceDirectory)) | ||
| { | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifest.Id.Value); | ||
| bool dirAlreadyExisted = Directory.Exists(contentDir); | ||
| Directory.CreateDirectory(contentDir); | ||
| contentDirCreatedByThisCall = !dirAlreadyExisted; | ||
|
|
||
| var sourcePathFile = Path.Combine(contentDir, "source.path"); | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
|
Check failure on line 507 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: The constant |
||
| await File.WriteAllTextAsync(sourcePathFile, sourceDirectory, cancellationToken); | ||
| sourcePathWrittenByThisCall = true; | ||
|
|
||
| _logger.LogInformation( | ||
| "Created source path mapping for {ManifestId}: {SourcePath}", | ||
|
|
@@ -534,6 +539,21 @@ | |
| { | ||
| FileOperationsService.DeleteFileIfExists(manifestPath); | ||
|
|
||
| // Clean up the content dir created for source.path mapping | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifest.Id.Value); | ||
| if (contentDirCreatedByThisCall) | ||
|
Check failure on line 544 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| { | ||
| // We created this directory in the current call β safe to remove entirely. | ||
| if (Directory.Exists(contentDir)) | ||
| Directory.Delete(contentDir, recursive: true); | ||
| } | ||
| else if (sourcePathWrittenByThisCall && Directory.Exists(contentDir)) | ||
|
Check failure on line 550 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| { | ||
| // Directory pre-existed but we wrote source.path this call β only remove that file. | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
|
Check failure on line 553 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| FileOperationsService.DeleteFileIfExists(sourcePathFile); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: The constant |
||
| } | ||
|
|
||
| // Untrack manifest if we failed to save its metadata but had already tracked/refreshed references. | ||
| await _referenceTracker.UntrackManifestAsync(manifest.Id, CancellationToken.None); | ||
| } | ||
|
|
@@ -793,4 +813,4 @@ | |
|
|
||
| return manifest; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CopyVersionToClipboardCommandis not defined β click does nothingThe
Buttonbinds to{Binding CopyVersionToClipboardCommand}, butMainViewModeldoes not define this command. I searched the entire codebase and found noCopyVersionToClipboardCommandanywhere β it only appears here in the XAML.At runtime the binding will silently fail: the button renders with a hand cursor and tooltip, but clicking it does nothing. You need to add a
[RelayCommand]method toMainViewModel.cs, for example:Prompt To Fix With AI