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

Add executed commands to shell history #112

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

llagerlof
Copy link
Contributor

No description provided.

@mefengl
Copy link

mefengl commented Jul 4, 2024

related: #99

if (historyFile) {
const lastCommand = getLastCommand(historyFile);
if (lastCommand !== command) {
fs.appendFile(historyFile, `${command}\n`, (err) => {
Copy link
Contributor

@steve8708 steve8708 Jul 8, 2024

Choose a reason for hiding this comment

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

should the newline come before the new command instead of after?

Suggested change
fs.appendFile(historyFile, `${command}\n`, (err) => {
fs.appendFile(historyFile, `\n${command}`, (err) => {

or is the idea that these files already always end in a newline (and if so are we 100% confident that this is the case all the time for all of the supported shells, or perhaps should we at least check, like

let fileContents = await readFile(historyFile)
if (!fileContents.endsWith('\n')) {
  // ensure we add the newline
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, but I added a newline at the end of the string because that's a POSIX rule. All lines in text files should end with a newline character, including the last line of plain text files. We can expect a newline at the end of all history files.

However, I am not opposed to checking it first.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah i see, works for me!

@steve8708 steve8708 merged commit 2d2c6bd into BuilderIO:main Jul 9, 2024
2 checks passed
@llagerlof llagerlof deleted the shell_history branch July 9, 2024 11:45
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.

3 participants