mirror of
https://github.com/Dictionarry-Hub/profilarr.git
synced 2026-01-22 10:51:02 +01:00
fix: merge/delete conflicts (#151)
feat: enhance merge conflict handling with improved status messages and conditional rendering feat: improved modify / delete conflict handling - fix ours/theirs messages for deleted files - fix handling of choice moniker - fix parsing of local/incoming name when deleted
This commit is contained in:
@@ -30,10 +30,12 @@ def get_version_data(repo, ref, file_path):
|
||||
|
||||
def resolve_conflicts(
|
||||
repo, resolutions: Dict[str, Dict[str, str]]) -> Dict[str, Any]:
|
||||
logger.debug(f"Received resolutions for files: {list(resolutions.keys())}")
|
||||
"""
|
||||
Resolve merge conflicts based on provided resolutions.
|
||||
"""
|
||||
logger.debug(f"Received resolutions for files: {list(resolutions.keys())}")
|
||||
logger.debug(f"Full resolutions data: {resolutions}")
|
||||
|
||||
try:
|
||||
status = repo.git.status('--porcelain', '-z').split('\0')
|
||||
conflicts = []
|
||||
@@ -82,6 +84,7 @@ def resolve_conflicts(
|
||||
if modify_delete_conflicts[file_path]:
|
||||
logger.debug(
|
||||
f"Handling modify/delete conflict for {file_path}")
|
||||
logger.debug(f"Field resolutions for modify/delete: {field_resolutions}")
|
||||
|
||||
# Get the existing version (either from HEAD or MERGE_HEAD)
|
||||
head_data = get_version_data(repo, 'HEAD', file_path)
|
||||
@@ -92,9 +95,17 @@ def resolve_conflicts(
|
||||
is_deleted_in_head = head_data is None
|
||||
existing_data = merge_head_data if is_deleted_in_head else head_data
|
||||
logger.debug(f"Existing version data: {existing_data}")
|
||||
logger.debug(f"is_deleted_in_head: {is_deleted_in_head}")
|
||||
logger.debug(f"head_data: {head_data}")
|
||||
logger.debug(f"merge_head_data: {merge_head_data}")
|
||||
|
||||
# Try both lowercase and capitalized versions of 'file'
|
||||
choice = field_resolutions.get('file') or field_resolutions.get('File')
|
||||
logger.debug(f"Resolution choice for file: {choice}")
|
||||
|
||||
choice = field_resolutions.get('file')
|
||||
if not choice:
|
||||
logger.error("No 'file' or 'File' resolution found in field_resolutions!")
|
||||
logger.error(f"Available keys: {list(field_resolutions.keys())}")
|
||||
raise Exception(
|
||||
"No resolution provided for modify/delete conflict")
|
||||
|
||||
@@ -153,9 +164,23 @@ def resolve_conflicts(
|
||||
ours_data = get_version_data(repo, 'HEAD', file_path)
|
||||
theirs_data = get_version_data(repo, 'MERGE_HEAD', file_path)
|
||||
|
||||
# For files that were previously involved in modify/delete conflicts
|
||||
# we may not be able to get all versions
|
||||
if not base_data or not ours_data or not theirs_data:
|
||||
raise Exception(
|
||||
f"Couldn't get all versions of {file_path}")
|
||||
logger.warning(f"Couldn't get all versions of {file_path} - may have been previously resolved as a modify/delete conflict")
|
||||
logger.warning(f"base_data: {base_data}, ours_data: {ours_data}, theirs_data: {theirs_data}")
|
||||
|
||||
# If it was previously resolved as "incoming" but ours_data is missing, use theirs_data
|
||||
if not ours_data and theirs_data:
|
||||
logger.info(f"Using incoming version for {file_path} as base for resolution")
|
||||
ours_data = theirs_data
|
||||
# If it was previously resolved as "local" but theirs_data is missing, use ours_data
|
||||
elif ours_data and not theirs_data:
|
||||
logger.info(f"Using local version for {file_path} as base for resolution")
|
||||
theirs_data = ours_data
|
||||
# If we can't recover either version, we can't proceed
|
||||
else:
|
||||
raise Exception(f"Couldn't get required versions of {file_path}")
|
||||
|
||||
# Start with a deep copy of ours_data to preserve all fields
|
||||
resolved_data = deepcopy(ours_data)
|
||||
|
||||
@@ -29,16 +29,20 @@ def compare_conflict_yaml(ours_data: Any,
|
||||
if ours_data is None and theirs_data is None:
|
||||
return conflicts
|
||||
if ours_data is None:
|
||||
# Local version deleted
|
||||
param_name = path or 'File'
|
||||
return [{
|
||||
'parameter': path or 'File',
|
||||
'local_value': 'deleted',
|
||||
'incoming_value': theirs_data
|
||||
'parameter': param_name,
|
||||
'local_value': '🗑️ File deleted in local version',
|
||||
'incoming_value': '📄 File exists in incoming version'
|
||||
}]
|
||||
if theirs_data is None:
|
||||
# Incoming version deleted
|
||||
param_name = path or 'File'
|
||||
return [{
|
||||
'parameter': path or 'File',
|
||||
'local_value': ours_data,
|
||||
'incoming_value': 'deleted'
|
||||
'parameter': param_name,
|
||||
'local_value': '📄 File exists in local version',
|
||||
'incoming_value': '🗑️ File deleted in incoming version'
|
||||
}]
|
||||
|
||||
# Handle different types as conflicts
|
||||
@@ -197,6 +201,7 @@ def create_conflict_summary(file_path: str,
|
||||
- file_path: Path to the conflicted file
|
||||
- type: Type of item
|
||||
- name: Name from our version or filename
|
||||
- incoming_name: Name from their version (if available)
|
||||
- status: Current conflict status
|
||||
- conflict_details: List of specific conflicts
|
||||
"""
|
||||
@@ -209,18 +214,37 @@ def create_conflict_summary(file_path: str,
|
||||
compare_conflict_yaml(ours_data, theirs_data)
|
||||
}
|
||||
|
||||
# Get name from our version or fallback to filename
|
||||
name = ours_data.get('name') if ours_data else os.path.basename(
|
||||
file_path)
|
||||
# Get local name
|
||||
local_name = None
|
||||
if ours_data and isinstance(ours_data, dict) and 'name' in ours_data:
|
||||
local_name = ours_data.get('name')
|
||||
|
||||
return {
|
||||
if not local_name:
|
||||
# Strip the extension to get a cleaner name
|
||||
basename = os.path.basename(file_path)
|
||||
local_name = os.path.splitext(basename)[0]
|
||||
|
||||
# Get incoming name
|
||||
incoming_name = None
|
||||
if theirs_data and isinstance(theirs_data, dict) and 'name' in theirs_data:
|
||||
incoming_name = theirs_data.get('name')
|
||||
|
||||
if not incoming_name:
|
||||
# Strip the extension to get a cleaner name
|
||||
basename = os.path.basename(file_path)
|
||||
incoming_name = os.path.splitext(basename)[0]
|
||||
|
||||
result = {
|
||||
'file_path': file_path,
|
||||
'type': determine_type(file_path),
|
||||
'name': name,
|
||||
'name': local_name,
|
||||
'incoming_name': incoming_name,
|
||||
'status': status,
|
||||
'conflict_details': conflict_details
|
||||
}
|
||||
|
||||
return result
|
||||
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"Failed to create conflict summary for {file_path}: {str(e)}")
|
||||
|
||||
@@ -31,20 +31,46 @@ def process_modify_delete_conflict(repo, file_path, deleted_in_head):
|
||||
else:
|
||||
status = MODIFY_DELETE
|
||||
|
||||
# Get the surviving version
|
||||
# For delete conflicts, we need to extract the name for display purposes
|
||||
# This will be the name of the actual file before it was deleted
|
||||
basename = os.path.basename(file_path)
|
||||
filename = os.path.splitext(basename)[0] # Strip extension
|
||||
|
||||
# Get metadata from existing version to extract name if possible
|
||||
if file_exists:
|
||||
with open(os.path.join(repo.working_dir, file_path), 'r') as f:
|
||||
existing_data = yaml.safe_load(f.read())
|
||||
# File exists locally, read it
|
||||
try:
|
||||
with open(os.path.join(repo.working_dir, file_path), 'r') as f:
|
||||
existing_data = yaml.safe_load(f.read())
|
||||
except Exception as read_error:
|
||||
logger.warning(f"Could not read existing file {file_path}: {str(read_error)}")
|
||||
existing_data = {'name': filename}
|
||||
else:
|
||||
existing_data = get_version_data(repo, 'MERGE_HEAD', file_path)
|
||||
# File was deleted locally, try to get from merge head
|
||||
try:
|
||||
existing_data = get_version_data(repo, 'MERGE_HEAD', file_path)
|
||||
except Exception as merge_error:
|
||||
logger.warning(f"Could not get merge head for {file_path}: {str(merge_error)}")
|
||||
existing_data = {'name': filename}
|
||||
|
||||
if not existing_data:
|
||||
return None
|
||||
# Simplified placeholder data for deleted version
|
||||
if deleted_in_head:
|
||||
# File was deleted in HEAD (local) but exists in MERGE_HEAD (incoming)
|
||||
local_data = None # This indicates deleted
|
||||
try:
|
||||
# Try to get name from incoming
|
||||
incoming_data = existing_data if existing_data else {'name': filename}
|
||||
except Exception:
|
||||
incoming_data = {'name': filename}
|
||||
else:
|
||||
# File exists in HEAD (local) but deleted in MERGE_HEAD (incoming)
|
||||
try:
|
||||
local_data = existing_data if existing_data else {'name': filename}
|
||||
except Exception:
|
||||
local_data = {'name': filename}
|
||||
incoming_data = None # This indicates deleted
|
||||
|
||||
# Create summary with the appropriate sides marked as deleted
|
||||
return create_conflict_summary(
|
||||
file_path, None if deleted_in_head else existing_data,
|
||||
existing_data if deleted_in_head else None, status)
|
||||
return create_conflict_summary(file_path, local_data, incoming_data, status)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
|
||||
@@ -15,9 +15,10 @@ const ConflictRow = ({change, fetchGitStatus}) => {
|
||||
param => param.parameter === 'name'
|
||||
);
|
||||
|
||||
// Check for name in conflicting parameters or directly in change object
|
||||
const displayLocalName =
|
||||
nameConflict?.local_value || change.name || 'Unnamed';
|
||||
const displayIncomingName = nameConflict?.incoming_value || 'Unnamed';
|
||||
const displayIncomingName = nameConflict?.incoming_value || change?.incoming_name || 'Unnamed';
|
||||
|
||||
const isResolved = change.status === 'RESOLVED';
|
||||
|
||||
|
||||
@@ -10,14 +10,22 @@ const MergeConflicts = ({
|
||||
areAllConflictsResolved,
|
||||
fetchGitStatus
|
||||
}) => {
|
||||
if (!conflicts || conflicts.length === 0) return null;
|
||||
const hasConflicts = conflicts && conflicts.length > 0;
|
||||
|
||||
return (
|
||||
<div className='mb-4'>
|
||||
<div className='flex items-center justify-between'>
|
||||
<h4 className='text-sm font-medium text-gray-200 flex items-center'>
|
||||
<AlertTriangle className='text-yellow-400 mr-2' size={16} />
|
||||
<span>Merge Conflicts</span>
|
||||
{areAllConflictsResolved() ? (
|
||||
<CheckCircle className='text-green-400 mr-2' size={16} />
|
||||
) : (
|
||||
<AlertTriangle className='text-yellow-400 mr-2' size={16} />
|
||||
)}
|
||||
<span>
|
||||
{areAllConflictsResolved()
|
||||
? 'All Conflicts Resolved'
|
||||
: 'Merge Conflicts'}
|
||||
</span>
|
||||
</h4>
|
||||
<div className='flex space-x-2'>
|
||||
<Tooltip
|
||||
@@ -47,10 +55,23 @@ const MergeConflicts = ({
|
||||
</Tooltip>
|
||||
</div>
|
||||
</div>
|
||||
<ConflictTable
|
||||
conflicts={conflicts}
|
||||
fetchGitStatus={fetchGitStatus}
|
||||
/>
|
||||
|
||||
{/* Only show the conflict table if there are conflicts */}
|
||||
{hasConflicts && (
|
||||
<ConflictTable
|
||||
conflicts={conflicts}
|
||||
fetchGitStatus={fetchGitStatus}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* Show a success message when all conflicts are resolved */}
|
||||
{!hasConflicts && areAllConflictsResolved() && (
|
||||
<div className='mt-3 p-4 bg-gray-800 border border-gray-700 rounded-lg text-center'>
|
||||
<p className='text-gray-300'>
|
||||
All conflicts have been successfully resolved. You can now commit the merge.
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user