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

Added Toggle Tail config in Debug. #373

Closed
wants to merge 0 commits into from
Closed

Conversation

sjoshid
Copy link

@sjoshid sjoshid commented Dec 12, 2018

@codecov-io
Copy link

Codecov Report

Merging #373 into master will increase coverage by 1.63%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
+ Coverage   41.59%   43.23%   +1.63%     
==========================================
  Files          36       36              
  Lines        4130     4138       +8     
==========================================
+ Hits         1718     1789      +71     
+ Misses       2412     2349      -63
Impacted Files Coverage Δ
Sources/XiEditor/XiCore.swift 89.74% <0%> (-4.86%) ⬇️
Sources/XiEditor/AppDelegate.swift 28.72% <0%> (+0.15%) ⬆️
Sources/XiEditor/RPCSending.swift 55.81% <0%> (-1.17%) ⬇️
Sources/XiEditor/EditView.swift 55.3% <0%> (+0.85%) ⬆️
Sources/XiEditor/XiTextPlane/Atlas.swift 78.57% <0%> (+1.78%) ⬆️
Sources/XiEditor/EditViewController.swift 37.61% <0%> (+10.39%) ⬆️
Sources/XiEditor/ClientImplementation.swift 49.56% <0%> (+10.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fb44c6...3c092a0. Read the comment docs.

Copy link
Member

@mmatoszko mmatoszko left a comment

Choose a reason for hiding this comment

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

Super nice and clear PR 👍

Sources/XiEditor/AppDelegate.swift Outdated Show resolved Hide resolved
Sources/XiEditor/XiCore.swift Outdated Show resolved Hide resolved
@jansol
Copy link
Contributor

jansol commented Dec 16, 2018

The function and RPC are called 'toggle' but what it does looks more like 'set' at a quick glance. This seems somewhat confusing to me. Maybe rename or explain the naming in a comment in the code?

@cmyr
Copy link
Member

cmyr commented Dec 16, 2018

This is tracking xi-editor/xi-editor#1011; the final shape of this will be determined by decisions there, so holding review until then.

@cmyr cmyr added the hold label Dec 16, 2018
@sjoshid
Copy link
Author

sjoshid commented Feb 3, 2019

@cmyr
Made some progress here. I have followed existing language changed implementation as it is similar in functionality to tail feature.
One thing remaining: disable the toggle tail menu when file doesnt exist.

@cmyr
Copy link
Member

cmyr commented Feb 8, 2019

@sjoshid This includes a bunch of files I don't think you intended?

A tip I find useful: after updating a PR (and before confirming to open a PR) click on the "files changed" tab and read through the diffs. I very often find mistakes I've made this way!

@sjoshid
Copy link
Author

sjoshid commented Feb 9, 2019

Oops. Sorry about that. Not sure how they sneaked in. I think I did a bunch of useless commits but the final version looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants