search

xi-editor / xi-mac

The xi-editor mac frontend.
Apache License 2.0
3.02k stars 147 forks source link

Add tail config #484

Closed sjoshid closed 4 years ago

sjoshid commented 4 years ago

Closes https://github.com/xi-editor/xi-editor/issues/922. This PR adds a "Tail File" option under Debug menu. This option can be enabled/disabled per file.

TODO: Gray out this option if the file doesnt exist. No point in tailing a file that doesnt exist.

Review Checklist

sjoshid commented 4 years ago

I highly recommend using the "Automatically trim trailing whitespace" feature in Xcode. The diff has a lot of unrelated whitespace changes, which can make it hard for reviewers to focus on the parts that matter.

When I started working on this we were targeting this to be like tail command. But Im open for automatic trimming. Will look in to this.

If I toggle tail on a file and I open another file, this causes a crash.

Hmm. I cannot replicate this issue. Can you please provide more details? Keep in mind if you try to tail a file that doesnt exists, it does crash. (See TODO in description) But not sure if this is what you are trying to do.

I do think tailing is a super cool feature that I'd like to have, since one of the things I commonly use xi for is to read logs

Same here. There arent any editors out there that do this efficiently, and frankly the way tail does it. Thanks for reviewing this. I'll keep updating this PR. Hopped on just to give an update and try the crash use case.

nangtrongvuon commented 4 years ago

I highly recommend using the "Automatically trim trailing whitespace" feature in Xcode. The diff has a lot of unrelated whitespace changes, which can make it hard for reviewers to focus on the parts that matter.

When I started working on this we were targeting this to be like tail command. But Im open for automatic trimming. Will look in to this.

Oh, I don't mean that the tail feature should trim whitespaces, but I was assuming you made this pull request in Xcode. I mean that you should turn on that setting in Xcode, so your diff doesn't have too many whitespace changes.

Other than that, looking forward to your tail work!

sjoshid commented 4 years ago

haha. silly me. Will get to this as soon as I get some time.

Thanks.

nangtrongvuon commented 4 years ago

Heya, I took another look at this, and the changes look good! Here are some of my comments on this PR before it can be merged:

sjoshid commented 4 years ago

Current CI is saying the PR doesn't build properly because core doesn't build properly, you might want to check the corresponding core commit.

Yeah. Core is build successfully yesterday. Is there a way to force build?

The PR's commit log is a bit messy. I suggest just extracting the current changes here and making just one commit that's based off the current upstream master tip.

I work on multiple machines so I commit everything I can which adds to the messyness. I will squash commits.

This is a just a big nit, but the whitespaces are still in the changelist. I think if you follow the "making a new commit advice", that should solve the whitespaces problem with the Xcode trim whitespace setting turned on as well.

I have the trim whitespace setting turned on for sure. I'll try the advice thing when I squash commits.

nangtrongvuon commented 4 years ago

You can force a rebuild simply by force pushing to this branch again, that's what I do all the time too. :P

sjoshid commented 4 years ago

@nangtrongvuon Two things 1) I squashed commits. Dont think there is a way to reflect that in this PR. I still see it says there are 23 commits but in fact there are only 4. This could be because I created this branch from my forked master where I hadnt squashed commits. :(. so maybe I can create a new PR from the new branch again? If I do, it'll show only 4 commits. 2) The build keeps failing. Any idea why? Here's my core PR https://github.com/xi-editor/xi-editor/pull/1241.

sjoshid commented 4 years ago

Im going to close this PR. There are too many unnecessary commits. I'll open a new PR with required changes.