Skip to content

Add --cxml flag#16

Merged
simonw merged 5 commits intosimonw:mainfrom
lexh:xml
Sep 9, 2024
Merged

Add --cxml flag#16
simonw merged 5 commits intosimonw:mainfrom
lexh:xml

Conversation

@lexh
Copy link
Copy Markdown
Contributor

@lexh lexh commented Apr 24, 2024

Closes #15

@jefftriplett
Copy link
Copy Markdown

This is great and saves me a ton of time! Thank you for opening the pull request.

@lexh
Copy link
Copy Markdown
Contributor Author

lexh commented Sep 4, 2024

FYI there is a degenerate behavior in this patch when no files are passed in.

> files-to-prompt --xml
</documents>

Not sure there is too much of an appetite for this to be merged in. Trying to read Simon's mind, the existing implementation probably gives comparable results while using less tokens and being more simple.

But I'll push another commit to better handle that case anyway.

@simonw
Copy link
Copy Markdown
Owner

simonw commented Sep 4, 2024

Yeah let’s fix that and ship this!

if not ignore_gitignore:
gitignore_rules.extend(read_gitignore(os.path.dirname(path)))
if xml and path == paths[0]:
click.echo("Here are some documents for you to reference for your task:")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don’t like including this prompt here

@simonw
Copy link
Copy Markdown
Owner

simonw commented Sep 7, 2024

The fact that this doesn’t guarantee a valid XML document (that could then be handled by an XML parser) really bugs me!

A few options:

  1. Have it produce valid XML, which mainly means applying correct escaping to text between the XML tags. I’m worried this may hurt Claude’s performance, we would need to test that but I’m not sure how best to do that.
  2. Admit that this isn’t valid XML and change the option to something like -c/--cxml (for Claude XML).
  3. Call this XML but have another option, --valid-xml - which does produce valid XML and by its very existence calls out the Claude not-valid-XML thing.

I’m leaning towards 2.

@jefftriplett
Copy link
Copy Markdown

  • Admit that this isn’t valid XML and change the option to something like -c/--cxml (for Claude XML).

This feels like a good compromise and opens the door for a valid --xml option later.

I have tested with both valid and Claude's XML, and I don't see any noticeable improvements either way. They have other features where randomly dropping in an xml tag has meaning in an otherwise other text sea of chaos.

@simonw simonw changed the title Add --xml flag Add --cxml flag Sep 9, 2024
@simonw simonw merged commit db4a164 into simonw:main Sep 9, 2024
simonw added a commit that referenced this pull request Sep 9, 2024
simonw added a commit that referenced this pull request Sep 9, 2024
cyber-pro1 added a commit to cyber-pro1/file-to-prompt that referenced this pull request Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add --xml flag to structure output for Claude's long context window

3 participants