Codealike / Codealike-Atom

MIT License
0 stars 1 forks source link

Add check for project path #17

Closed iKlem closed 5 years ago

iKlem commented 5 years ago

Hi.

I'm using Codealike to have a way to track my personal work time and I'm working with two computers : one with Windows 10 and another one with Manjaro (Arch Linux).

The problem is when i want to work on my Manjaro, the codealike package isn't working.

In the package, to configure the project inside Codealike (if I understand), you tried to get the atom.project.rootDirectories["0"].lowerCasePath but I've found that there's no lowerCasePath attribute :

before_fix

This pull request add the condition to check if we have the lowerCasePath attribute and if we not, we put the path one.

After doing the check I've got this :

after_fix

As you can see, the package has the Codealike is tracking on-line.

I haven't tested on Mac since I do not own one.

diglesias17 commented 5 years ago

Hi Clément, In first place, thank you for providing us with an issue description and a feasible solution. We really appreciate that!

We analyzed your pull request and discovered that it would not work because you are not using the atomProject variable to configure the Codealike instance.

Anyway we have worked on a solution and it would be great if you can confirm it works as expected on your linux instance.

Would you confirm that this version of the startTrackingProject() method works for you?

startTrackingProject() {
    let paths = atom.project.getPaths();

    // verify if there is a loaded folder in workspace
    if (paths.length) {
      // then ensure codealike configuration exists
      // and start tracking
      let rootPath = paths[0].toLowerCase();

      Codealike
        .configure(rootPath)
        .then(
          (configuration) => {
            // calculate when workspace started loading
            let currentDate = new Date();
            let startupDurationInMs = atom.getWindowLoadTime();
            currentDate.setMilliseconds(currentDate.getMilliseconds()-startupDurationInMs);

            // start tracking project
            Codealike.startTracking(configuration, currentDate);
          }
        );
    }
  },

If you confirms it works, we will release a new version of the atom plugin with the fix as soon as possible.

Once again. Thank you for your cooperation!!!

iKlem commented 5 years ago

Hi.

Oops for the atomProject I had missed ^^.

I'll try this patch tomorrow then.

iKlem commented 5 years ago

So I've done some tests with your patch but it doesn't work with my Manjaro. But if I remove the .toLowerCase function, it create the codealike.json file and track correctly and i think why : In linux, path is case sensitive but in Windows it's case insensitive. The code works, but on linux, you'll need to get the path without lowering the case.

So one question : why lowering the case for the path ?

diglesias17 commented 5 years ago

That is a very good question. Perhaps it is not completely necessary. I will review if that conversion to lower case is really required.

In the meantime if you want to send the pull request with the version you have working we would be glad to integrate your aport to our code base :)

Thanks again for your help in this matter.

Obtener Outlook para Androidhttps://aka.ms/ghei36


From: Clément Delalande notifications@github.com Sent: Wednesday, February 6, 2019 3:28:26 PM To: Codealike/Codealike-Atom Cc: Daniel Iglesias; Comment Subject: Re: [Codealike/Codealike-Atom] Add check for project path (#17)

So I've done some tests with your patch but it doesn't work with my Manjaro. But if I remove the .toLowerCase function, it create the codealike.json file and track correctly and i think why : In linux, path is case sensitive but in Windows it's case insensitive. The code works, but on linux, you'll need to get the path wihtout lowering the case.

So one question : why lowering the case for the path ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FCodealike%2FCodealike-Atom%2Fpull%2F17%23issuecomment-461133392&data=02%7C01%7C%7C153553fdb2474923566c08d68c60e2cb%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636850745073703883&sdata=olH8dCtPJu0lvY3H8FC7ZxiGzJFYlJy685fcMpmxY8o%3D&reserved=0, or mute the threadhttps://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADROOIJODZHVSMSlauQu7z5G6tJDqma6ks5vKx7KgaJpZM4aj3sc&data=02%7C01%7C%7C153553fdb2474923566c08d68c60e2cb%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636850745073713900&sdata=Nr15EcKs1HO%2BAlCtscmfEv2IqY3N4MywdBm0G4QZ8uo%3D&reserved=0.

iKlem commented 5 years ago

The last commit reflect the patch you gave without the toLowerCase function.

Tested on Windows 10 & Manjaro (arch linux).