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

Reset the file path after clear the buffer #228

Merged
merged 4 commits into from
Aug 28, 2024
Merged

Conversation

ccqpein
Copy link
Contributor

@ccqpein ccqpein commented Aug 24, 2024

Try to implement the issue #227 .

After (setf shell-maker-if-file-path-reset-after-clean 'reset) and manually binding the ("C-c M-o" . chatgpt-shell-clear-buffer). It will reset the shell-maker--fileto nil after I clean the buffer. So I can save the new conversation in new file.

**Feel free to change it if it isn't match the design of owner.

Copy link
Owner

@xenodium xenodium left a comment

Choose a reason for hiding this comment

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

Thanks for the draft!

Left a few comments. Also, be sure to run M-x checkdoc and byte compile the files (can do it from dired via M-x dired-do-byte-compile).

I'm aware of a few warnings in the compilation. Let's try not to add more ;)

@@ -607,6 +605,13 @@ Set NEW-SESSION to start a separate new session."
#'chatgpt-shell-prompt-compose)
shell-buffer))

(defun chatgpt-shell-clear-buffer ()
Copy link
Owner

Choose a reason for hiding this comment

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

We should consider moving this to shell-maker since there's nothing chatgpt-shell specific.

chatgpt-shell.el Outdated Show resolved Hide resolved
shell-maker.el Outdated Show resolved Hide resolved
shell-maker.el Outdated Show resolved Hide resolved
shell-maker.el Outdated Show resolved Hide resolved
shell-maker.el Outdated Show resolved Hide resolved
Copy link
Owner

@xenodium xenodium left a comment

Choose a reason for hiding this comment

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

Thank you for the changes! Left some minor comments if you're keen to address. Otherwise, I'll follow up.

chatgpt-shell.el Outdated
@@ -607,6 +605,13 @@ Set NEW-SESSION to start a separate new session."
#'chatgpt-shell-prompt-compose)
shell-buffer))

(defun chatgpt-shell-clear-buffer ()
"Clean the current shell buffer."
Copy link
Owner

Choose a reason for hiding this comment

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

nit: Clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just give the new commit.

shell-maker.el Outdated
@@ -97,6 +97,11 @@ For example:
:type 'directory
:group 'shell-maker)

(defcustom shell-maker-forget-file-after-clean nil
"Tell the `shell-maker-save-session-transcript' reset file path or not."
Copy link
Owner

Choose a reason for hiding this comment

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

nit: Consider "If non-nil, reset file path after clear command." as other booleans in this file.

ps. Heads-up, I've just made 54f3166 which is more idiomatic.

Copy link
Owner

@xenodium xenodium left a comment

Choose a reason for hiding this comment

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

Thanks for the changes and the PR!

@xenodium xenodium merged commit 68b6c5a into xenodium:main Aug 28, 2024
@ccqpein ccqpein changed the title Draft: Reset the file path after clean the buffer Reset the file path after clean the buffer Aug 28, 2024
@ccqpein
Copy link
Contributor Author

ccqpein commented Aug 28, 2024

No problem

@xenodium
Copy link
Owner

A couple of follow-ups:

  • We need to move it to shell-maker to make it compatible with the "clear" shell command: a342d33
  • Typo: 2f55255

@xenodium xenodium changed the title Reset the file path after clean the buffer Reset the file path after clear the buffer Aug 28, 2024
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.

2 participants