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.

Sign up for GitHub

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

Jump to bottom

Add XDG Specification compliance #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add XDG Specification compliance #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions bm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ var fs = require('fs');
var argv = require('minimist')(process.argv.slice(2));

var userhome = process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'];
var rc_path = userhome+"/.bmndrrc";
Copy link

Choose a reason for hiding this comment

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

I see these are all vars and maybe that is standard javascript. 🤷🏿
At first glance a const seems more appropriate.

Copy link
Author

Choose a reason for hiding this comment

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

I think this was more about maintaining consistency with the rest of the file.

if (process.env.XDG_CONFIG_HOME) {
var rc_path = process.env.XDG_CONFIG_HOME+"/.bmndrrc";
Copy link

Choose a reason for hiding this comment

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

DRY?
How about instead calculating the "rc_path_prefix" using the logic from this if else and then appending the local folder to that, just once?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds like a good approach. How would you implement that?

Copy link

Choose a reason for hiding this comment

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

Upon second thought ...

While it makes sense to start the folder off with a period when it is directly in the home directory - so as to have it default hidden, doing so while under XDG_CONFIG_HOME (which should point to $HOME/.config unless configured otherwise) for the same reason makes less sense.

$HOME/.config is the hidden folder and it subfolders need not be to avoid cluttering a listing of $HOME.

Copy link
Author

Choose a reason for hiding this comment

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

So change
var rc_path = process.env.XDG_CONFIG_HOME+"/.bmndrrc";
to
var rc_path = process.env.XDG_CONFIG_HOME+"/bmndrrc";
?

} else {
var rc_path = userhome+"/.bmndrrc";
Copy link

Choose a reason for hiding this comment

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

As far as I can tell, userhome is now only being used here on L13. If so, its scope could be reduced.

Copy link
Author

@quintrino quintrino May 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As in something like this?

if (process.env.XDG_CONFIG_HOME) {
  var rc_path = process.env.XDG_CONFIG_HOME+"/.bmndrrc";
} else {
  let userhome = process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'];
  var rc_path = userhome+"/.bmndrrc";
}

}


var exists = fs.existsSync(rc_path);
var rc = exists && fs.readFileSync(rc_path, 'utf8');
Expand Down Expand Up @@ -193,7 +198,7 @@ if (argv.help) {
printHelp();
}
} else {
console.log("No ~/.bmndrrc detected... starting authentication process...");
console.log(`No ${rc_path} detected... starting authentication process...`);
var open;
if (process.platform === 'linux') {
open = 'xdg-open';
Expand All @@ -209,7 +214,7 @@ if (argv.help) {
if (err) { return onErr(err); }
fs.writeFile(rc_path, "[account]\nauth_token: "+result.auth_token+"\n", function (errf) {
if (errf) { return onErr(errf); }
console.log("Successfully wrote auth_token to ~/.bmndrrc");
console.log(`Successfully wrote auth_token to ${rc_path}`);
});
});
});
Expand Down